larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

remove readErr and writeErr from operations (#2587)

Browse Source 
## Description

Now that fault is in place, we can remove the
readErrs and writeErrs from operation persistence.

## Does this PR need a docs update or release note?

- [x] ⛔ No 

## Type of change

- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1970

## Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-02-27 12:28:28 -07:00 committed by GitHub
parent b5c6c77b62
commit 3a2203ded5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 26 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/internal/operations/backup.go
View File

@ -6,7 +6,6 @@ import (
"github.com/alcionai/clues"
"github.com/google/uuid"
multierror "github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common"
@ -93,10 +92,9 @@ func (op BackupOperation) validate() error {
// pointer wrapping the values, while those values
// get populated asynchronously.
type backupStats struct {
k *kopia.BackupStats
gc *support.ConnectorOperationStatus
resourceCount int
readErr, writeErr error
k *kopia.BackupStats
gc *support.ConnectorOperationStatus
resourceCount int
}
type detailsWriter interface {
@ -166,7 +164,6 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) {
With("err", err).
Errorw("doing backup", clues.InErr(err).Slice()...)
op.Errors.Fail(errors.Wrap(err, "doing backup"))
opStats.readErr = op.Errors.Failure()
}
// TODO: the consumer (sdk or cli) should run this, not operations.
@ -185,8 +182,6 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) {
err = op.persistResults(startTime, &opStats)
if err != nil {
op.Errors.Fail(errors.Wrap(err, "persisting backup results"))
opStats.writeErr = op.Errors.Failure()
return op.Errors.Failure()
}
@ -212,8 +207,6 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) {
deets.Details())
if err != nil {
op.Errors.Fail(errors.Wrap(err, "persisting backup"))
opStats.writeErr = op.Errors.Failure()
return op.Errors.Failure()
}
@ -628,19 +621,11 @@ func (op *BackupOperation) persistResults(
) error {
op.Results.StartedAt = started
op.Results.CompletedAt = time.Now()
op.Results.ReadErrors = opStats.readErr
op.Results.WriteErrors = opStats.writeErr
op.Status = Completed
if opStats.readErr != nil || opStats.writeErr != nil {
if op.Errors.Failure() != nil {
op.Status = Failed
// TODO(keepers): replace with fault.Errors handling.
return multierror.Append(
errors.New("errors prevented the operation from processing"),
opStats.readErr,
opStats.writeErr)
}
op.Results.BytesRead = opStats.k.TotalHashedBytes
@ -653,13 +638,13 @@ func (op *BackupOperation) persistResults(
return errors.New("backup population never completed")
}
if opStats.gc.Metrics.Successes == 0 {
if op.Status != Failed && opStats.gc.Metrics.Successes == 0 {
op.Status = NoData
}
op.Results.ItemsRead = opStats.gc.Metrics.Successes
return nil
return op.Errors.Failure()
}
// stores the operation details, results, and selectors in the backup manifest.

12
src/internal/operations/backup_test.go
View File

@ -379,6 +379,7 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() {
expectStatus opStatus
expectErr assert.ErrorAssertionFunc
stats backupStats
fail error
}{
{
expectStatus: Completed,
@ -398,10 +399,10 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() {
{
expectStatus: Failed,
expectErr: assert.Error,
fail: assert.AnError,
stats: backupStats{
readErr: assert.AnError,
k: &kopia.BackupStats{},
gc: &support.ConnectorOperationStatus{},
k: &kopia.BackupStats{},
gc: &support.ConnectorOperationStatus{},
},
},
{
@ -428,6 +429,9 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() {
sel,
evmock.NewBus())
require.NoError(t, err)
op.Errors.Fail(test.fail)
test.expectErr(t, op.persistResults(now, &test.stats))
assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status")
@ -436,8 +440,6 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() {
assert.Equal(t, test.stats.k.TotalHashedBytes, op.Results.BytesRead, "bytes read")
assert.Equal(t, test.stats.k.TotalUploadedBytes, op.Results.BytesUploaded, "bytes written")
assert.Equal(t, test.stats.resourceCount, op.Results.ResourceOwners, "resource owners")
assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors")
assert.Equal(t, test.stats.writeErr, op.Results.WriteErrors, "write errors")
assert.Equal(t, now, op.Results.StartedAt, "started at")
assert.Less(t, now, op.Results.CompletedAt, "completed at")
})

31
src/internal/operations/restore.go
View File

@ -8,7 +8,6 @@ import (
"github.com/alcionai/clues"
"github.com/google/uuid"
multierror "github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common"
@ -88,11 +87,10 @@ func (op RestoreOperation) validate() error {
// pointer wrapping the values, while those values
// get populated asynchronously.
type restoreStats struct {
cs []data.RestoreCollection
gc *support.ConnectorOperationStatus
bytesRead *stats.ByteCounter
resourceCount int
readErr, writeErr error
cs []data.RestoreCollection
gc *support.ConnectorOperationStatus
bytesRead *stats.ByteCounter
resourceCount int
// a transient value only used to pair up start-end events.
restoreID string
@ -153,7 +151,6 @@ func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.De
With("err", err).
Errorw("doing restore", clues.InErr(err).Slice()...)
op.Errors.Fail(errors.Wrap(err, "doing restore"))
opStats.readErr = op.Errors.Failure()
}
// TODO: the consumer (sdk or cli) should run this, not operations.
@ -172,8 +169,6 @@ func (op *RestoreOperation) Run(ctx context.Context) (restoreDetails *details.De
err = op.persistResults(ctx, start, &opStats)
if err != nil {
op.Errors.Fail(errors.Wrap(err, "persisting restore results"))
opStats.writeErr = op.Errors.Failure()
return nil, op.Errors.Failure()
}
@ -277,19 +272,11 @@ func (op *RestoreOperation) persistResults(
) error {
op.Results.StartedAt = started
op.Results.CompletedAt = time.Now()
op.Results.ReadErrors = opStats.readErr
op.Results.WriteErrors = opStats.writeErr
op.Status = Completed
if opStats.readErr != nil || opStats.writeErr != nil {
if op.Errors.Failure() != nil {
op.Status = Failed
// TODO(keepers): replace with fault.Errors handling.
return multierror.Append(
errors.New("errors prevented the operation from processing"),
opStats.readErr,
opStats.writeErr)
}
op.Results.BytesRead = opStats.bytesRead.NumBytes
@ -301,21 +288,19 @@ func (op *RestoreOperation) persistResults(
return errors.New("restoration never completed")
}
if opStats.gc.Metrics.Successes == 0 {
if op.Status != Failed && opStats.gc.Metrics.Successes == 0 {
op.Status = NoData
}
op.Results.ItemsWritten = opStats.gc.Metrics.Successes
dur := op.Results.CompletedAt.Sub(op.Results.StartedAt)
op.bus.Event(
ctx,
events.RestoreEnd,
map[string]any{
events.BackupID: op.BackupID,
events.DataRetrieved: op.Results.BytesRead,
events.Duration: dur,
events.Duration: op.Results.CompletedAt.Sub(op.Results.StartedAt),
events.EndTime: common.FormatTime(op.Results.CompletedAt),
events.ItemsRead: op.Results.ItemsRead,
events.ItemsWritten: op.Results.ItemsWritten,
@ -327,7 +312,7 @@ func (op *RestoreOperation) persistResults(
},
)
return nil
return op.Errors.Failure()
}
// formatDetailsForRestoration reduces the provided detail entries according to the

8
src/internal/operations/restore_test.go
View File

@ -53,6 +53,7 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() {
expectStatus opStatus
expectErr assert.ErrorAssertionFunc
stats restoreStats
fail error
}{
{
expectStatus: Completed,
@ -78,8 +79,8 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() {
{
expectStatus: Failed,
expectErr: assert.Error,
fail: assert.AnError,
stats: restoreStats{
readErr: assert.AnError,
bytesRead: &stats.ByteCounter{},
gc: &support.ConnectorOperationStatus{},
},
@ -109,6 +110,9 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() {
dest,
evmock.NewBus())
require.NoError(t, err)
op.Errors.Fail(test.fail)
test.expectErr(t, op.persistResults(ctx, now, &test.stats))
assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status")
@ -116,8 +120,6 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() {
assert.Equal(t, test.stats.gc.Metrics.Successes, op.Results.ItemsWritten, "items written")
assert.Equal(t, test.stats.bytesRead.NumBytes, op.Results.BytesRead, "resource owners")
assert.Equal(t, test.stats.resourceCount, op.Results.ResourceOwners, "resource owners")
assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors")
assert.Equal(t, test.stats.writeErr, op.Results.WriteErrors, "write errors")
assert.Equal(t, now, op.Results.StartedAt, "started at")
assert.Less(t, now, op.Results.CompletedAt, "completed at")
})