feat(execution): Exclude execution retrieval for Disabled Front50 pipelines #4819
Conversation
orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/Front50Service.groovy
Outdated
Show resolved
Hide resolved
christosarvanitis
force-pushed
the
feat/excludeExecutionForDisabledPipelines
branch
from
5c86e68 to
f849250
Compare
christosarvanitis
force-pushed
the
feat/excludeExecutionForDisabledPipelines
branch
from
f849250 to
086a55d
Compare
| @Component | ||
| @ConditionalOnExpression( | ||
| "${pollers.unused-pipelines-disable.enabled:false} && ${execution-repository.sql.enabled:false}") | ||
| public class UnusedPipelineDisablePollingNotificationAgent |
There was a problem hiding this comment.
javadoc please. What does this class do, and why do we need it?
christosarvanitis
force-pushed
the
feat/excludeExecutionForDisabledPipelines
branch
from
e5a4857 to
412524c
Compare
| @Nonnull criteria: ExecutionCriteria | ||
| ): List<String> { | ||
| return ( | ||
| primary.retrievePipelineConfigIdsForApplicationWithCriteria(application, criteria) + |
There was a problem hiding this comment.
Think his needs a .distinct()...
There was a problem hiding this comment.
Good catch! Done
| .filter(p -> !orcaExecutionsPipelineConfigIds.contains(p)) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| log.info( |
There was a problem hiding this comment.
Does this need to be a .info level log?
There was a problem hiding this comment.
I switched to info only in DryRun mode and on normal operation to debug.
| criteria: ExecutionCriteria | ||
| ): List<String> { | ||
| var baseQueryPredicate = field("application").eq(application) | ||
| var table = if (jooq.dialect() == SQLDialect.MYSQL) PIPELINE.tableName.forceIndex("pipeline_application_idx") |
There was a problem hiding this comment.
As a rule, try to avoid force indexes - let the database optimize as needed as if new indexes come online or old ones get removed this would require work here.
There was a problem hiding this comment.
Following the optimised retrieval from this PR #4804 I can revert to a simple query. What do you think?
| @@ -248,7 +248,7 @@ class TaskControllerSpec extends Specification { | |||
| pipelineConfigId = config.pipelineConfigId | |||
| } | |||
| }) | |||
| front50Service.getPipelines(app, false) >> [[id: "1"], [id: "2"]] | |||
| front50Service.getPipelines(app, false, null) >> [[id: "1"], [id: "2"]] | |||
There was a problem hiding this comment.
I see null tests, but be nice to verify that it's reading the values & passing them downstream as planned. Not "critical" though.
There was a problem hiding this comment.
Sure, pushed an updated test that shows every scenario
christosarvanitis
force-pushed
the
feat/excludeExecutionForDisabledPipelines
branch
2 times, most recently
from
a838fe7 to
1a280e1
Compare
christosarvanitis
force-pushed
the
feat/excludeExecutionForDisabledPipelines
branch
from
1a280e1 to
c3e3f22
Compare
Adding an opt-in feature to exclude Execution Retrieval for Disabled Front50 pipelines. This can be configured via:
When enabled Orca calls the Front50 with a query parameter
enabledPipelines=true. Front50 responds only with the enabled pipelines of the application.This feature aims to ease the load on the execution retrieval for applications that have a lot of pipelines and for historical reasons users dont want to delete the obsolete pipelines that are disabled.
Additionally an agent is implemented to check for unused/unexecuted pipelines per application for the past X days and disable them in Front50. This is only applicable for SQL execution repository and can be configured via:
Related Front50 change spinnaker/front50#1520