Skip to content

Commit

Permalink
Merge pull request #1029 from alfrunes/perf-deployment-status
Browse files Browse the repository at this point in the history 
perf: Optimize database interactions when updating deployment stats
  • Loading branch information
@alfrunes
alfrunes authored Jul 4, 2024
2 parents 2dd2eed + 382cd62 commit 10c12f3
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 171 deletions.
127 changes: 72 additions & 55 deletions app/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1397,20 +1397,28 @@ func (d *Deployments) createDeviceDeploymentWithStatus(
return nil, err
}

// after inserting new device deployment update deployment stats
// in the database and locally, and update deployment status
if err := d.db.UpdateStatsInc(
ctx, deployment.Id,
prevStatus, status,
); err != nil {
return nil, err
}

deployment.Stats.Inc(status)

err = d.recalcDeploymentStatus(ctx, deployment)
if err != nil {
return nil, errors.Wrap(err, "failed to update deployment status")
if prevStatus != status {
beforeStatus := deployment.GetStatus()
// after inserting new device deployment update deployment stats
// in the database, and update deployment status
deployment.Stats, err = d.db.UpdateStatsInc(
ctx, deployment.Id,
prevStatus, status,
)
if err != nil {
return nil, err
}
newStatus := deployment.GetStatus()
if beforeStatus != newStatus {
err = d.db.SetDeploymentStatus(
ctx, deployment.Id,
newStatus, time.Now(),
)
if err != nil {
return nil, errors.Wrap(err,
"failed to update deployment status")
}
}
}

if !status.Active() {
Expand Down Expand Up @@ -1554,7 +1562,7 @@ func (d *Deployments) saveDeviceDeploymentRequest(ctx context.Context, deviceID
"old data: %s",
deviceID, request, deviceDeployment.Request)

if err := d.UpdateDeviceDeploymentStatus(ctx, deviceDeployment.DeploymentId, deviceID,
if err := d.updateDeviceDeploymentStatus(ctx, deviceDeployment,
model.DeviceDeploymentState{
Status: model.DeviceDeploymentStatusFailure,
}); err != nil {
Expand All @@ -1580,28 +1588,43 @@ func (d *Deployments) saveDeviceDeploymentRequest(ctx context.Context, deviceID
return nil
}

// UpdateDeviceDeploymentStatus will update the deployment status for device of
// updateDeviceDeploymentStatus will update the deployment status for device of
// ID `deviceID`. Returns nil if update was successful.
func (d *Deployments) UpdateDeviceDeploymentStatus(ctx context.Context, deploymentID string,
deviceID string, ddState model.DeviceDeploymentState) error {
func (d *Deployments) UpdateDeviceDeploymentStatus(
ctx context.Context,
deviceID, deploymentID string,
ddState model.DeviceDeploymentState,
) error {
deviceDeployment, err := d.db.GetDeviceDeployment(
ctx, deviceID, deploymentID, false,
)
if err == mongo.ErrStorageNotFound {
return ErrStorageNotFound
} else if err != nil {
return err
}

return d.updateDeviceDeploymentStatus(ctx, deviceDeployment, ddState)
}

func (d *Deployments) updateDeviceDeploymentStatus(
ctx context.Context,
dd *model.DeviceDeployment,
ddState model.DeviceDeploymentState,
) error {

l := log.FromContext(ctx)

l.Infof("New status: %s for device %s deployment: %v", ddState.Status, deviceID, deploymentID)
l.Infof("New status: %s for device %s deployment: %v",
ddState.Status, dd.DeviceId, dd.DeploymentId,
)

var finishTime *time.Time = nil
if model.IsDeviceDeploymentStatusFinished(ddState.Status) {
now := time.Now()
finishTime = &now
}

dd, err := d.db.GetDeviceDeployment(ctx, deploymentID, deviceID, false)
if err == mongo.ErrStorageNotFound {
return ErrStorageNotFound
} else if err != nil {
return err
}

currentStatus := dd.Status

if currentStatus == model.DeviceDeploymentStatusAborted {
Expand All @@ -1621,24 +1644,30 @@ func (d *Deployments) UpdateDeviceDeploymentStatus(ctx context.Context, deployme
ddState.FinishTime = finishTime

old, err := d.db.UpdateDeviceDeploymentStatus(ctx,
deviceID, deploymentID, ddState, dd.Status)
dd.DeviceId, dd.DeploymentId, ddState, dd.Status)
if err != nil {
return err
}

if err = d.db.UpdateStatsInc(ctx, deploymentID, old, ddState.Status); err != nil {
return err
}

// fetch deployment stats and update deployment status
deployment, err := d.db.FindDeploymentByID(ctx, deploymentID)
if err != nil {
return errors.Wrap(err, "failed when searching for deployment")
}
if old != ddState.Status {
// fetch deployment stats and update deployment status
deployment, err := d.db.FindDeploymentByID(ctx, dd.DeploymentId)
if err != nil {
return errors.Wrap(err, "failed when searching for deployment")
}
beforeStatus := deployment.GetStatus()

err = d.recalcDeploymentStatus(ctx, deployment)
if err != nil {
return errors.Wrap(err, "failed to update deployment status")
deployment.Stats, err = d.db.UpdateStatsInc(ctx, dd.DeploymentId, old, ddState.Status)
if err != nil {
return err
}
newStatus := deployment.GetStatus()
if beforeStatus != newStatus {
err = d.db.SetDeploymentStatus(ctx, dd.DeploymentId, newStatus, time.Now())
if err != nil {
return errors.Wrap(err, "failed to update deployment status")
}
}
}

if !ddState.Status.Active() {
Expand All @@ -1652,7 +1681,7 @@ func (d *Deployments) UpdateDeviceDeploymentStatus(ctx context.Context, deployme
if err := d.db.SaveLastDeviceDeploymentStatus(ctx, ldd); err != nil {
l.Error(errors.Wrap(err, "failed to save last device deployment status").Error())
}
if err := d.reindexDevice(ctx, deviceID); err != nil {
if err := d.reindexDevice(ctx, dd.DeviceId); err != nil {
l.Warn(errors.Wrap(err, "failed to trigger a device reindex"))
}
if err := d.reindexDeployment(ctx, dd.DeviceId, dd.DeploymentId, dd.Id); err != nil {
Expand All @@ -1663,19 +1692,6 @@ func (d *Deployments) UpdateDeviceDeploymentStatus(ctx context.Context, deployme
return nil
}

// recalcDeploymentStatus inspects the deployment stats and
// recalculates and updates its status
// it should be used whenever deployment stats are touched
func (d *Deployments) recalcDeploymentStatus(ctx context.Context, dep *model.Deployment) error {
status := dep.GetStatus()

if err := d.db.SetDeploymentStatus(ctx, dep.Id, status, time.Now()); err != nil {
return err
}

return nil
}

func (d *Deployments) GetDeploymentStats(ctx context.Context,
deploymentID string) (model.Stats, error) {

Expand Down Expand Up @@ -1925,8 +1941,9 @@ func (d *Deployments) updateDeviceDeploymentsStatus(
Status: status,
FinishTime: &now,
}
if err := d.UpdateDeviceDeploymentStatus(ctx, deviceDeployment.DeploymentId,
deviceId, ddStatus); err != nil {
if err := d.updateDeviceDeploymentStatus(
ctx, deviceDeployment, ddStatus,
); err != nil {
return errors.Wrap(err, "updating device deployment status")
}
latestDeployment = deviceDeployment.Created
Expand Down
8 changes: 3 additions & 5 deletions app/app_os.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ func (d *Deployments) handleAlreadyInstalled(
deviceDeployment *model.DeviceDeployment,
) error {
l := log.FromContext(ctx)
if err := d.UpdateDeviceDeploymentStatus(
if err := d.updateDeviceDeploymentStatus(
ctx,
deviceDeployment.DeploymentId,
deviceDeployment.DeviceId,
deviceDeployment,
model.DeviceDeploymentState{
Status: model.DeviceDeploymentStatusAlreadyInst,
}); err != nil {
Expand Down Expand Up @@ -137,8 +136,7 @@ func (d *Deployments) assignNoArtifact(
deviceDeployment *model.DeviceDeployment,
) error {
l := log.FromContext(ctx)
if err := d.UpdateDeviceDeploymentStatus(ctx, deviceDeployment.DeploymentId,
deviceDeployment.DeviceId,
if err := d.updateDeviceDeploymentStatus(ctx, deviceDeployment,
model.DeviceDeploymentState{
Status: model.DeviceDeploymentStatusNoArtifact,
}); err != nil {
Expand Down
45 changes: 38 additions & 7 deletions app/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ func TestUpdateDeviceDeploymentStatus(t *testing.T) {
db.On("UpdateStatsInc", ctx,
fakeDeployment.Id,
model.DeviceDeploymentStatusDownloading,
model.DeviceDeploymentStatusInstalling).Return(nil).Once()

// fake updated stats
fakeDeployment.Stats.Set(model.DeviceDeploymentStatusInstalling, 1)
model.DeviceDeploymentStatusInstalling).Run(func(args mock.Arguments) {
// fake updated stats
fakeDeployment.Stats.Inc(model.DeviceDeploymentStatusInstalling)
}).Return(fakeDeployment.Stats, nil).Once()

db.On("FindDeploymentByID", ctx, fakeDeployment.Id).Return(
fakeDeployment, nil).Once()
Expand Down Expand Up @@ -176,7 +176,10 @@ func TestGetDeploymentForDeviceWithCurrent(t *testing.T) {
db.On("UpdateStatsInc", ctx,
fakeDeployment.Id,
model.DeviceDeploymentStatusPending,
model.DeviceDeploymentStatusAlreadyInst).Return(nil)
model.DeviceDeploymentStatusAlreadyInst).Run(func(args mock.Arguments) {
// fake updated stats
fakeDeployment.Stats.Inc(model.DeviceDeploymentStatusAlreadyInst)
}).Return(fakeDeployment.Stats, nil).Once()

// fake updated stats
fakeDeployment.Stats.Set(model.DeviceDeploymentStatusNoArtifact, 1)
Expand Down Expand Up @@ -255,11 +258,13 @@ func TestDecommissionDevice(t *testing.T) {

findOldestDeploymentForDeviceIDWithStatusesDeployment: &model.DeviceDeployment{
Id: "bar",
DeviceId: "foo",
DeploymentId: "bar",
Status: model.DeviceDeploymentStatusDownloading,
},
getDeviceDeploymentDeployment: &model.DeviceDeployment{
Id: "bar",
DeviceId: "foo",
DeploymentId: "bar",
Status: model.DeviceDeploymentStatusDownloading,
},
Expand Down Expand Up @@ -358,9 +363,20 @@ func TestDecommissionDevice(t *testing.T) {
db.On("FindDeploymentByID", ctx, tc.inputDeploymentId).Return(
tc.findDeploymentByIDDeployment, tc.findDeploymentByIDError)

var stats model.Stats
if tc.findDeploymentByIDDeployment != nil {
stats = tc.findDeploymentByIDDeployment.Stats
}
db.On("UpdateStatsInc", ctx, tc.inputDeploymentId,
tc.updateDeviceDeploymentStatusStatus,
model.DeviceDeploymentStatusDecommissioned).Return(tc.updateStatsIncError)
model.DeviceDeploymentStatusDecommissioned).
Run(func(args mock.Arguments) {
if stats != nil {
stats.Inc(model.DeviceDeploymentStatusDecommissioned)
}
}).
Return(stats, tc.updateStatsIncError).
Once()

db.On("SetDeploymentStatus", ctx,
tc.inputDeploymentId,
Expand Down Expand Up @@ -430,11 +446,13 @@ func TestAbortDeviceDeployments(t *testing.T) {

findOldestDeploymentForDeviceIDWithStatusesDeployment: &model.DeviceDeployment{
Id: "bar",
DeviceId: "foo",
DeploymentId: "bar",
Status: model.DeviceDeploymentStatusDownloading,
},
getDeviceDeploymentDeployment: &model.DeviceDeployment{
Id: "bar",
DeviceId: "foo",
DeploymentId: "bar",
Status: model.DeviceDeploymentStatusDownloading,
},
Expand Down Expand Up @@ -548,9 +566,22 @@ func TestAbortDeviceDeployments(t *testing.T) {
db.On("FindDeploymentByID", ctx, tc.inputDeploymentId).Return(
tc.findDeploymentByIDDeployment, tc.findDeploymentByIDError)

var stats model.Stats
if tc.findDeploymentByIDDeployment != nil {
stats = tc.findDeploymentByIDDeployment.Stats
}
db.On("UpdateStatsInc", ctx, tc.inputDeploymentId,
tc.updateDeviceDeploymentStatusStatus,
model.DeviceDeploymentStatusAborted).Return(tc.updateStatsIncError)
model.DeviceDeploymentStatusAborted).
Run(func(args mock.Arguments) {
if stats != nil {
stats.Inc(model.DeviceDeploymentStatusAborted)
}
}).
Return(
stats,
tc.updateStatsIncError,
)

status := model.DeploymentStatusFinished
if tc.isDeploymentInProgress {
Expand Down
2 changes: 1 addition & 1 deletion store/datastore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ type DataStore interface {
id string,
stateFrom,
stateTo model.DeviceDeploymentStatus,
) error
) (model.Stats, error)
UpdateStats(ctx context.Context,
id string, stats model.Stats) error
Find(ctx context.Context,
Expand Down
21 changes: 15 additions & 6 deletions store/mocks/DataStore.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 10c12f3

Please sign in to comment.