Skip to content

Commit 1815677

Browse files
authored
Merge pull request #6516 from devtron-labs/fix-chart-crud-mem-issue
fix: sql query memory issue
2 parents 37ee8a6 + a4e8a7a commit 1815677

File tree

7 files changed

+39
-15
lines changed

7 files changed

+39
-15
lines changed

cmd/external-app/wire_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

env_gen.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

env_gen.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@
168168
| DEX_CSTOREKEY | string | | | | false |
169169
| DEX_JWTKEY | string | | | | false |
170170
| DEX_RURL | string |http://127.0.0.1:8080/callback | | | false |
171+
| DEX_SCOPES | | | | | false |
171172
| DEX_SECRET | string | | | | false |
172173
| DEX_URL | string | | | | false |
173174
| ECR_REPO_NAME_PREFIX | string |test/ | | | false |

internal/sql/repository/chartConfig/EnvConfigOverrideRepository.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ type EnvConfigOverrideRepository interface {
7373
UpdateWithTxn(envConfigOverride *EnvConfigOverride, tx *pg.Tx) (*EnvConfigOverride, error)
7474

7575
GetByAppIdEnvIdAndChartRefId(appId, envId int, chartRefId int) (*EnvConfigOverride, error)
76-
GetAllOverridesForApp(appId int) ([]EnvConfigOverride, error)
76+
// GetAllOverridesForApp will return all overrides []*EnvConfigOverride for an app by appId
77+
// Note:
78+
// EnvConfigOverride.Chart is not populated,
79+
// as the chartRepoRepository.Chart contains the reference chart(in bytes).
80+
GetAllOverridesForApp(appId int) ([]*EnvConfigOverride, error)
7781
}
7882

7983
type EnvConfigOverrideRepositoryImpl struct {
@@ -360,13 +364,19 @@ func (r EnvConfigOverrideRepositoryImpl) GetByAppIdEnvIdAndChartRefId(appId, env
360364
return eco, err
361365
}
362366

363-
func (r EnvConfigOverrideRepositoryImpl) GetAllOverridesForApp(appId int) ([]EnvConfigOverride, error) {
364-
var eco []EnvConfigOverride
367+
// GetAllOverridesForApp will return all overrides EnvConfigOverride for an app by appId
368+
// Note:
369+
// EnvConfigOverride.Chart is not populated,
370+
// as the chartRepoRepository.Chart contains the reference chart(in bytes).
371+
func (r EnvConfigOverrideRepositoryImpl) GetAllOverridesForApp(appId int) ([]*EnvConfigOverride, error) {
372+
var eco []*EnvConfigOverride
365373
err := r.dbConnection.
366374
Model(&eco).
367-
Column("env_config_override.*", "Chart").
375+
Column("env_config_override.*").
376+
Join("INNER JOIN charts").
377+
JoinOn("charts.id = env_config_override.chart_id").
368378
Where("env_config_override.active = ?", true).
369-
Where("Chart.app_id =? ", appId).
379+
Where("charts.app_id = ?", appId).
370380
Select()
371381
return eco, err
372382
}

pkg/chart/ChartService.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ func (impl *ChartServiceImpl) Create(templateRequest bean3.TemplateRequest, ctx
315315
return nil, err
316316
}
317317

318-
err = impl.UpdateChartLocationForEnvironmentConfigs(templateRequest.AppId, chart.ChartRefId, templateRequest.UserId, version)
318+
err = impl.updateChartLocationForEnvironmentConfigs(newCtx, templateRequest.AppId, chart.ChartRefId, templateRequest.UserId, version)
319319
if err != nil {
320320
impl.logger.Errorw("error in updating chart location in env overrides", "appId", templateRequest.AppId, "err", err)
321321
return nil, err
@@ -350,8 +350,10 @@ func (impl *ChartServiceImpl) Create(templateRequest bean3.TemplateRequest, ctx
350350
return chartVal, err
351351
}
352352

353-
func (impl *ChartServiceImpl) UpdateChartLocationForEnvironmentConfigs(appId, chartRefId int, userId int32, version string) error {
354-
envOverrides, err := impl.envConfigOverrideReadService.GetAllOverridesForApp(appId)
353+
func (impl *ChartServiceImpl) updateChartLocationForEnvironmentConfigs(ctx context.Context, appId, chartRefId int, userId int32, version string) error {
354+
newCtx, span := otel.Tracer("orchestrator").Start(ctx, "ChartServiceImpl.updateChartLocationForEnvironmentConfigs")
355+
defer span.End()
356+
envOverrides, err := impl.envConfigOverrideReadService.GetAllOverridesForApp(newCtx, appId)
355357
if err != nil {
356358
impl.logger.Errorw("error in getting all overrides for app", "appId", appId, "err", err)
357359
return err
@@ -661,7 +663,7 @@ func (impl *ChartServiceImpl) UpdateAppOverride(ctx context.Context, templateReq
661663
return nil, err
662664
}
663665

664-
err = impl.UpdateChartLocationForEnvironmentConfigs(templateRequest.AppId, templateRequest.ChartRefId, templateRequest.UserId, template.ChartVersion)
666+
err = impl.updateChartLocationForEnvironmentConfigs(newCtx, templateRequest.AppId, templateRequest.ChartRefId, templateRequest.UserId, template.ChartVersion)
665667
if err != nil {
666668
impl.logger.Errorw("error in updating chart location in env overrides", "appId", templateRequest.AppId, "err", err)
667669
return nil, err

pkg/deployment/manifest/deploymentTemplate/read/chartEnvConfigOverride.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package read
22

33
import (
4+
"context"
45
"github.com/devtron-labs/devtron/internal/sql/repository/chartConfig"
56
"github.com/devtron-labs/devtron/pkg/deployment/manifest/deploymentTemplate/adapter"
67
"github.com/devtron-labs/devtron/pkg/deployment/manifest/deploymentTemplate/bean"
8+
"go.opentelemetry.io/otel"
79
"go.uber.org/zap"
810
)
911

@@ -18,7 +20,11 @@ type EnvConfigOverrideService interface {
1820
FindChartByAppIdAndEnvIdAndChartRefId(appId, targetEnvironmentId int, chartRefId int) (*bean.EnvConfigOverride, error)
1921
FindChartForAppByAppIdAndEnvId(appId, targetEnvironmentId int) (*bean.EnvConfigOverride, error)
2022
GetByAppIdEnvIdAndChartRefId(appId, envId int, chartRefId int) (*bean.EnvConfigOverride, error)
21-
GetAllOverridesForApp(appId int) ([]*bean.EnvConfigOverride, error)
23+
// GetAllOverridesForApp will return all overrides []*bean.EnvConfigOverride for an app by appId
24+
// Note:
25+
// EnvConfigOverride.Chart is not populated,
26+
// as the chartRepoRepository.Chart contains the reference chart(in bytes).
27+
GetAllOverridesForApp(ctx context.Context, appId int) ([]*bean.EnvConfigOverride, error)
2228
}
2329

2430
type EnvConfigOverrideReadServiceImpl struct {
@@ -132,15 +138,20 @@ func (impl EnvConfigOverrideReadServiceImpl) GetByAppIdEnvIdAndChartRefId(appId,
132138
return adapter.EnvOverrideDBToDTO(overrideDBObj), nil
133139
}
134140

135-
func (impl EnvConfigOverrideReadServiceImpl) GetAllOverridesForApp(appId int) ([]*bean.EnvConfigOverride, error) {
141+
func (impl EnvConfigOverrideReadServiceImpl) GetAllOverridesForApp(ctx context.Context, appId int) ([]*bean.EnvConfigOverride, error) {
142+
_, span := otel.Tracer("orchestrator").Start(ctx, "EnvConfigOverrideReadServiceImpl.GetAllOverridesForApp")
143+
defer span.End()
136144
overrideDBObjs, err := impl.envConfigOverrideRepository.GetAllOverridesForApp(appId)
137145
if err != nil {
138146
impl.logger.Errorw("error in getting chart env config override", "appId", appId, "envId", "err", err)
139147
return nil, err
140148
}
141149
envConfigOverrides := make([]*bean.EnvConfigOverride, 0, len(overrideDBObjs))
142150
for _, dbObj := range overrideDBObjs {
143-
envConfigOverrides = append(envConfigOverrides, adapter.EnvOverrideDBToDTO(&dbObj))
151+
if dbObj == nil {
152+
continue // nil pointer handling
153+
}
154+
envConfigOverrides = append(envConfigOverrides, adapter.EnvOverrideDBToDTO(dbObj))
144155
}
145156
return envConfigOverrides, nil
146157
}

wire_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)