Minor snapshot lookup refactor and logging addition (#2164)

## Description

Make some parts of the lookup logic more uniform as to when reasons are added to things.

Add logging to make the process easier to debug

## 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

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1675
* #2149 

## Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-01-18 13:41:58 -08:00 committed by GitHub
parent e3b6d035fb
commit 383b93f097
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -117,6 +117,7 @@ func getLastIdx(
// adds it to the returned list. Otherwise no incomplete manifest is returned.
// Returns nil if there are no complete or incomplete manifests in mans.
func manifestsSinceLastComplete(
ctx context.Context,
mans []*snapshot.Manifest,
) ([]*snapshot.Manifest, bool) {
var (
@ -134,9 +135,10 @@ func manifestsSinceLastComplete(
if len(m.IncompleteReason) > 0 {
if !foundIncomplete {
res = append(res, m)
foundIncomplete = true
res = append(res, m)
logger.Ctx(ctx).Infow("found incomplete snapshot", "snapshot_id", m.ID)
}
continue
@ -147,6 +149,8 @@ func manifestsSinceLastComplete(
res = append(res, m)
foundComplete = true
logger.Ctx(ctx).Infow("found complete snapshot", "snapshot_id", m.ID)
break
}
@ -164,7 +168,7 @@ func fetchPrevManifests(
foundMans map[manifest.ID]*ManifestEntry,
reason Reason,
tags map[string]string,
) ([]*ManifestEntry, error) {
) ([]*snapshot.Manifest, error) {
allTags := map[string]string{}
for _, k := range reason.TagKeys() {
@ -188,8 +192,7 @@ func fetchPrevManifests(
// We have a complete cached snapshot and it's the most recent. No need
// to do anything else.
if lastCompleteIdx == len(metas)-1 {
man.Reasons = append(man.Reasons, reason)
return nil, nil
return []*snapshot.Manifest{man.Manifest}, nil
}
// TODO(ashmrtn): Remainder of the function can be simplified if we can inject
@ -209,24 +212,21 @@ func fetchPrevManifests(
return nil, errors.Wrap(err, "fetching previous manifests")
}
found, hasCompleted := manifestsSinceLastComplete(mans)
res := make([]*ManifestEntry, 0, len(found))
for _, m := range found {
res = append(res, &ManifestEntry{
Manifest: m,
Reasons: []Reason{reason},
})
}
found, hasCompleted := manifestsSinceLastComplete(ctx, mans)
// If we didn't find another complete manifest then we need to mark the
// previous complete manifest as having this ResourceOwner, Service, Category
// as the reason as well.
if !hasCompleted && man != nil {
man.Reasons = append(man.Reasons, reason)
found = append(found, man.Manifest)
logger.Ctx(ctx).Infow(
"reusing cached complete snapshot",
"snapshot_id",
man.ID,
)
}
return res, nil
return found, nil
}
// fetchPrevSnapshotManifests returns a set of manifests for complete and maybe
@ -252,6 +252,14 @@ func fetchPrevSnapshotManifests(
// we can pass in. Can be expanded to return more than the most recent
// snapshots, but may require more memory at runtime.
for _, reason := range reasons {
logger.Ctx(ctx).Infow(
"searching for previous manifests for reason",
"service",
reason.Service.String(),
"category",
reason.Category.String(),
)
found, err := fetchPrevManifests(
ctx,
sm,
@ -278,18 +286,16 @@ func fetchPrevSnapshotManifests(
for _, m := range found {
man := mans[m.ID]
if man == nil {
mans[m.ID] = m
mans[m.ID] = &ManifestEntry{
Manifest: m,
Reasons: []Reason{reason},
}
continue
}
// If the manifest already exists and it's incomplete then we should
// merge the reasons for consistency. This will become easier to handle
// once we update how checkpoint manifests are tagged.
if len(man.IncompleteReason) == 0 {
continue
}
man.Reasons = append(man.Reasons, m.Reasons...)
// This manifest has multiple reasons for being chosen. Merge them here.
man.Reasons = append(man.Reasons, reason)
}
}