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.

Sign up for GitHub

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

Jump to bottom

Improve the sql storage performance by using bulk save api #6147

Closed
sanopsmx opened this issue Nov 1, 2020 · 8 comments
Closed

Improve the sql storage performance by using bulk save api #6147

sanopsmx opened this issue Nov 1, 2020 · 8 comments
Labels
component/front50 sig/none Issues that do not fall under scope of any SIG stale to-be-closed

Comments

@sanopsmx
Copy link

sanopsmx commented Nov 1, 2020

Issue Summary:

Today, pipelines are saved serially one after the other in front50 sql RDBMS storage. If we have many pipelines to save, then it is taking lot of time to save these pipelines serially. This reduces the front50 save api performance.

Cloud Provider(s):

N/A

Environment:

Enable sql.storage=true in front 50 for sql storage.

Feature Area:

sig/security

This involves gate, orca and front50.

Description:

When we have multiple pipelines to save , we need to call front50 save api serially one after the other. This reduces the performance of front50 save api.

Steps to Reproduce:

Enable sql.enabled = true in front50-local.yml.
Install Mysql database.
Save multiple pipelines from gate api.

Additional Details:

Suggested Solution:

This is valid only for sql storage.

We improve the front50 save performance by using bulk insert query. This will help save many pipelines at once which greatly improves the performance of front50. We need to write a bulk save api which accepts an list of pipelines in a single rest api call and save them in front50 using bulk insert query.
@sanopsmx
Copy link
Author

sanopsmx commented Nov 1, 2020

I am working on this issue.

This was referenced Nov 1, 2020
Closed
Open
Closed
Closed
@ajordens
Copy link
Contributor

ajordens commented Nov 1, 2020

Curious, how many pipelines in a row are you saving that it's a "performance" problem?

Note that there is some prior art for doing bulk imports in front50 but it's only available for administrators. We've used this for doing behind-the-scenes migrations.

@maggieneterval maggieneterval added component/front50 sig/none Issues that do not fall under scope of any SIG labels Nov 2, 2020
@sanopsmx sanopsmx mentioned this issue Nov 2, 2020
Closed
This was referenced Nov 11, 2020
Closed
Open
Closed
clanesf pushed a commit to clanesf/orca that referenced this issue Dec 13, 2020
@sanopsmx
feat(pipeline executions/orca) : Added code to save multiple pipeline…
3c9393d 
…s at once to sql database.

This is part of: spinnaker/spinnaker#6147.

Enhanced SavePipelineTask.java to

Added code to ensure that the SavePipelineTask.java also accepts list of pipelines json.
This method will validate all the pipelines.
This method will call the front50 service to save the pipelines list.

Enhanced Front50Service.groovy to

Added new rest api which accepts list of pipelines json.
@spinnakerbot
Copy link

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

@sanopsmx
Copy link
Author

@spinnakerbot remove-label stale

@spinnakerbot
Copy link

"stale" has not been applied, and cannot be removed.

@spinnakerbot
Copy link

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

@spinnakerbot
Copy link

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

@spinnakerbot
Copy link

This issue is tagged as 'to-be-closed' and hasn't been updated in 45 days, so we are closing it. You can always reopen this issue if needed.

apoorvmahajan pushed a commit to apoorvmahajan/front50 that referenced this issue Aug 29, 2022
@sanopsmx
feat(pipeline executions/front50) : Added code to save multiple pipel…
23bcb18 
…ines at once to sql db.

This is part of: spinnaker/spinnaker#6147.

Enhanced PipelineController.java to

Added new rest api method bulksave(List<Map> pipelineList, Boolean staleCheck)
This method accepts a list of pipelines json.
This method checks each and every pipeline for authorization.
If the pipeline has valid authorization, then it will be saved to a savedPipelineList.
If the pipeline do not have a valid authorization, then it will be saved to a errorPipelineList
 and an error message is also populated.
All the savedPipelinesList will be saved to sql database using bulk insert.
This method returns a Map object having the below data:
{
  “Successful”: <count>,
  “Failed”: <cound>,
  “Failed_list”: [<array of failed pipelines - (application, pipeline name, etc) and the error message]
}

Enhanced AuthorizationSupport.java to

Added code to accept the pipeline list and authorize the pipelines.

Enhanced PipelineControllerSpec.java to

Added bean of FiatPermissionEvalutor.java as it is used in the PipelineController constructor.
apoorvmahajan pushed a commit to apoorvmahajan/gate that referenced this issue Aug 29, 2022
@sanopsmx
feat(pipeline executions/gate) : Added code to save multiple pipeline…
8be4237 
…s at once to sql db.

This is part of: spinnaker/spinnaker#6147.

Enhanced PipelineController.groovy to

Added new rest api method bulksavePipeline(List<Map> pipelineList, Boolean staleCheck)
This method accepts a list of pipelines json.
This method returns a Map object having the below data:
{
  “Successful”: <count>,
  “Failed”: <cound>,
  “Failed_list”: [<array of failed pipelines - (application, pipeline name, etc) and the error message]
}

Enhanced TaskService.java to

Added code to poll the task until the SavePipelineTask and MonitorFront50Task status : succeeded, terminal.
dbyron-sf pushed a commit to dbyron-sf/front50 that referenced this issue Apr 7, 2024
@sanopsmx @dbyron-sf
feat(pipeline executions/front50) : Added code to save multiple pipel…
43f367c 
…ines at once to sql db.

This is part of: spinnaker/spinnaker#6147.

Enhanced PipelineController.java to

Added new rest api method bulksave(List<Map> pipelineList, Boolean staleCheck)
This method accepts a list of pipelines json.
This method checks each and every pipeline for authorization.
If the pipeline has valid authorization, then it will be saved to a savedPipelineList.
If the pipeline do not have a valid authorization, then it will be saved to a errorPipelineList
 and an error message is also populated.
All the savedPipelinesList will be saved to sql database using bulk insert.
This method returns a Map object having the below data:
{
  “Successful”: <count>,
  “Failed”: <cound>,
  “Failed_list”: [<array of failed pipelines - (application, pipeline name, etc) and the error message]
}

Enhanced AuthorizationSupport.java to

Added code to accept the pipeline list and authorize the pipelines.

Enhanced PipelineControllerSpec.java to

Added bean of FiatPermissionEvalutor.java as it is used in the PipelineController constructor.

Conflicts:
	front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/PipelineController.java
	front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerSpec.groovy
	front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerTck.groovy
due to additional imports from spinnaker#1251 and spinnaker#1252

and org.springframework.context.support.DefaultMessageSourceResolvable has been removed
from master by spinnaker#1035

note, this fails to compile because bulkSave has code that expects Pipelines to behave as
Maps.  As of spinnaker#1035 that's no longer true.
dbyron-sf pushed a commit to dbyron-sf/front50 that referenced this issue Apr 18, 2024
@sanopsmx @dbyron-sf
feat(pipeline executions/front50) : Added code to save multiple pipel…
e2d81e8 
…ines at once to sql db.

This is part of: spinnaker/spinnaker#6147.

Enhanced PipelineController.java to

Added new rest api method bulksave(List<Map> pipelineList, Boolean staleCheck)
This method accepts a list of pipelines json.
This method checks each and every pipeline for authorization.
If the pipeline has valid authorization, then it will be saved to a savedPipelineList.
If the pipeline do not have a valid authorization, then it will be saved to a errorPipelineList
 and an error message is also populated.
All the savedPipelinesList will be saved to sql database using bulk insert.
This method returns a Map object having the below data:
{
  “Successful”: <count>,
  “Failed”: <cound>,
  “Failed_list”: [<array of failed pipelines - (application, pipeline name, etc) and the error message]
}

Enhanced AuthorizationSupport.java to

Added code to accept the pipeline list and authorize the pipelines.

Enhanced PipelineControllerSpec.java to

Added bean of FiatPermissionEvalutor.java as it is used in the PipelineController constructor.

Conflicts:
	front50-web/src/main/java/com/netflix/spinnaker/front50/controllers/PipelineController.java
	front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerSpec.groovy
	front50-web/src/test/groovy/com/netflix/spinnaker/front50/controllers/PipelineControllerTck.groovy
due to additional imports from spinnaker#1251 and spinnaker#1252

and org.springframework.context.support.DefaultMessageSourceResolvable has been removed
from master by spinnaker#1035

note, this fails to compile because bulkSave has code that expects Pipelines to behave as
Maps.  As of spinnaker#1035 that's no longer true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component/front50 sig/none Issues that do not fall under scope of any SIG stale to-be-closed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ajordens @maggieneterval @spinnakerbot @sanopsmx