Skip group mailbox items that were deleted in flight (#4939)
<!-- PR description-->
Discovered an issue with how we were handling items deleted in flight with the lazy item reader. This needs to go in before integration PR. https://github.com/alcionai/corso/pull/4921
Here are the details.
**Orig:**
* For such items, we were using the exchange approach to persist an empty data file (with serialized version) and not persist it in details file.
* Exchange discovers the deleted items during next backup during delta query and excludes them.
**Problem:**
* Group mailbox doesn't support delta queries. Also, there is no graph API to discover/fetch deleted items.
* So if we use the exchange lazy reader approach like above, the ongoing backup will succeed. However, the next incremental backup fails during details merge with ` running backup: merging details: incomplete migration of backup details`.
* Failure is [here](c58cd9302f/src/internal/operations/backup.go (L837)). It's because the deleted item reporefs are still present in the merge base. Since there is no delta query, there is no `GetDeleted()` to exclude the item.
**Mod:**
* Using the approach drive lazy reader code uses to mark an item as skipped, so that kopia doesn't error out during upload.
---
#### Does this PR need a docs update or release note?
- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No
#### Type of change
<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup
#### Issue(s)
<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* https://github.com/alcionai/corso/issues/4862
#### Test Plan
<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E
This commit is contained in:
parent
aa876cabb9
commit
abc5ec1942
@ -4,6 +4,7 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
@ -402,15 +403,14 @@ func (lig *lazyItemGetter[C, I]) GetData(
|
||||
lig.containerIDs,
|
||||
lig.itemID)
|
||||
if err != nil {
|
||||
// If an item was deleted then return an empty file so we don't fail
|
||||
// the backup. Also return delInFlight as true so that kopia skips
|
||||
// adding ItemInfo to details.
|
||||
//
|
||||
// The item will be deleted from kopia on the next backup when the
|
||||
// delta token shows it's removed.
|
||||
if graph.IsErrDeletedInFlight(err) {
|
||||
logger.CtxErr(ctx, err).Info("item not found")
|
||||
return nil, nil, true, nil
|
||||
// For items that were deleted in flight, add the skip label so that
|
||||
// they don't lead to recoverable failures during backup.
|
||||
if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || graph.IsErrDeletedInFlight(err) {
|
||||
logger.CtxErr(ctx, err).Info("item deleted in flight. skipping")
|
||||
|
||||
// Returning delInFlight as true here for correctness, although the caller is going
|
||||
// to ignore it since we are returning an error.
|
||||
return nil, nil, true, clues.Wrap(err, "deleted item").Label(graph.LabelsSkippable)
|
||||
}
|
||||
|
||||
err = clues.WrapWC(ctx, err, "getting item data").Label(fault.LabelForceNoBackupCreation)
|
||||
|
||||
@ -551,19 +551,8 @@ func (suite *CollectionUnitSuite) TestLazyItem_ReturnsEmptyReaderOnDeletedInFlig
|
||||
li.ModTime(),
|
||||
"item mod time")
|
||||
|
||||
r, err := readers.NewVersionedRestoreReader(li.ToReader())
|
||||
require.NoError(t, err, clues.ToCore(err))
|
||||
|
||||
assert.Equal(t, readers.DefaultSerializationVersion, r.Format().Version)
|
||||
assert.True(t, r.Format().DelInFlight)
|
||||
|
||||
readData, err := io.ReadAll(r)
|
||||
assert.NoError(t, err, "reading item data: %v", clues.ToCore(err))
|
||||
|
||||
assert.Empty(t, readData, "read item data")
|
||||
|
||||
_, err = li.Info()
|
||||
assert.ErrorIs(t, err, data.ErrNotFound, "Info() error")
|
||||
_, err := readers.NewVersionedRestoreReader(li.ToReader())
|
||||
assert.ErrorIs(t, err, graph.ErrDeletedInFlight, "item should be marked deleted in flight")
|
||||
}
|
||||
|
||||
func (suite *CollectionUnitSuite) TestLazyItem() {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user