-
Notifications
You must be signed in to change notification settings - Fork 328
Expose internal scheduling metadata #2873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e0de59d
0351463
58f62fe
172d8fb
af997d5
dab370e
e55b31b
055b4e0
d216050
0c64dd7
8375d6e
0402551
8779771
a28185d
7abe525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1185,6 +1185,8 @@ func (s *Sandbox) Pause( | |
| } | ||
| cleanup.AddNoContext(ctx, rootfsDiff.Close) | ||
|
|
||
| schedulingMetadata := NewSnapshotSchedulingMetadata(originalMemfile.Header(), originalRootfs.Header()) | ||
|
cursor[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a pause/checkpoint creates dirty memory or rootfs data, this computes the response from the original template headers, so the newly created Useful? React with 👍 / 👎. |
||
|
|
||
| metadataFileLink := template.NewLocalFileLink(cachePaths.CacheMetadata()) | ||
| cleanup.AddNoContext(ctx, metadataFileLink.Close) | ||
|
|
||
|
|
@@ -1194,14 +1196,15 @@ func (s *Sandbox) Pause( | |
| } | ||
|
|
||
| return &Snapshot{ | ||
| Snapfile: snapfile, | ||
| Metafile: metadataFileLink, | ||
| MemfileDiff: memfileDiff, | ||
| MemfileDiffHeader: memfileDiffHeader, | ||
| RootfsDiff: rootfsDiff, | ||
| RootfsDiffHeader: NewResolvedDiffHeader(rootfsHeader), | ||
| MemfileBlockSize: originalMemfile.Header().Metadata.BlockSize, | ||
| RootfsBlockSize: originalRootfs.Header().Metadata.BlockSize, | ||
| Snapfile: snapfile, | ||
| Metafile: metadataFileLink, | ||
| MemfileDiff: memfileDiff, | ||
| MemfileDiffHeader: memfileDiffHeader, | ||
| RootfsDiff: rootfsDiff, | ||
| RootfsDiffHeader: NewResolvedDiffHeader(rootfsHeader), | ||
| SchedulingMetadata: schedulingMetadata, | ||
| MemfileBlockSize: originalMemfile.Header().Metadata.BlockSize, | ||
| RootfsBlockSize: originalRootfs.Header().Metadata.BlockSize, | ||
|
|
||
| BuildID: buildID, | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,9 @@ import ( | |
| "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block" | ||
| blockmetrics "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/block/metrics" | ||
| "github.com/e2b-dev/infra/packages/orchestrator/pkg/sandbox/build" | ||
| "github.com/e2b-dev/infra/packages/orchestrator/pkg/scheduling" | ||
| "github.com/e2b-dev/infra/packages/orchestrator/pkg/template/metadata" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/grpc/orchestrator" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/logger" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/storage" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" | ||
|
|
@@ -270,6 +272,16 @@ func (t *storageTemplate) Files() storage.CachePaths { | |
| return t.paths | ||
| } | ||
|
|
||
| func (t *storageTemplate) SchedulingMetadata() *orchestrator.SchedulingMetadata { | ||
| memfileHeader, memfileErr := t.memfileHeader.Result() | ||
| rootfsHeader, rootfsErr := t.rootfsHeader.Result() | ||
|
Comment on lines
+276
to
+277
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a build enters the cache via Useful? React with 👍 / 👎. |
||
| if memfileErr != nil || rootfsErr != nil { | ||
| return nil | ||
| } | ||
|
|
||
| return scheduling.FromHeaders(memfileHeader, rootfsHeader) | ||
|
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| func (t *storageTemplate) Memfile(ctx context.Context) (block.ReadonlyDevice, error) { | ||
| _, span := tracer.Start(ctx, "storage-template-memfile") | ||
| defer span.End() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| package scheduling | ||
|
|
||
| import ( | ||
| "cmp" | ||
| "slices" | ||
|
|
||
| "github.com/google/uuid" | ||
|
|
||
| "github.com/e2b-dev/infra/packages/shared/pkg/grpc/orchestrator" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" | ||
| ) | ||
|
|
||
| const chainLimit = 32 | ||
|
|
||
| type buildContribution struct { | ||
| buildID uuid.UUID | ||
| memfileBytes uint64 | ||
| memfileMappingCount uint32 | ||
| rootfsBytes uint64 | ||
| rootfsMappingCount uint32 | ||
| } | ||
|
|
||
| func FromHeaders(memfileHeader, rootfsHeader *header.Header) *orchestrator.SchedulingMetadata { | ||
| if memfileHeader == nil || memfileHeader.Metadata == nil || rootfsHeader == nil || rootfsHeader.Metadata == nil { | ||
| return nil | ||
| } | ||
|
|
||
| contributions := make(map[uuid.UUID]*buildContribution) | ||
| add := func(buildID uuid.UUID, length uint64, isMemfile bool) { | ||
| if buildID == uuid.Nil || length == 0 { | ||
| return | ||
| } | ||
| c, ok := contributions[buildID] | ||
| if !ok { | ||
| c = &buildContribution{buildID: buildID} | ||
| contributions[buildID] = c | ||
| } | ||
| if isMemfile { | ||
| c.memfileBytes += length | ||
| c.memfileMappingCount++ | ||
| } else { | ||
| c.rootfsBytes += length | ||
| c.rootfsMappingCount++ | ||
| } | ||
| } | ||
|
|
||
| for _, m := range memfileHeader.Mapping.All() { | ||
| add(m.BuildId, m.Length, true) | ||
| } | ||
| for _, m := range rootfsHeader.Mapping.All() { | ||
| add(m.BuildId, m.Length, false) | ||
| } | ||
|
|
||
| baseBuildID := memfileHeader.Metadata.BaseBuildId | ||
| if baseBuildID == uuid.Nil { | ||
| baseBuildID = rootfsHeader.Metadata.BaseBuildId | ||
| } | ||
|
|
||
| chain := make([]buildContribution, 0, len(contributions)) | ||
| for _, c := range contributions { | ||
| chain = append(chain, *c) | ||
| } | ||
| slices.SortFunc(chain, func(a, b buildContribution) int { | ||
| aBytes := a.memfileBytes + a.rootfsBytes | ||
| bBytes := b.memfileBytes + b.rootfsBytes | ||
| if aBytes == bBytes { | ||
| return cmp.Compare(a.buildID.String(), b.buildID.String()) | ||
| } | ||
|
|
||
| return cmp.Compare(bBytes, aBytes) | ||
| }) | ||
| if len(chain) > chainLimit { | ||
| chain = chain[:chainLimit] | ||
| } | ||
|
|
||
| res := &orchestrator.SchedulingMetadata{ | ||
| BaseBuildId: baseBuildID.String(), | ||
| Generation: max(memfileHeader.Metadata.Generation, rootfsHeader.Metadata.Generation) + 1, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This helper is also called by Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generation incremented twiceHigh Severity
Additional Locations (2)Reviewed by Cursor Bugbot for commit d216050. Configure here. |
||
| MemfileSize: memfileHeader.Metadata.Size, | ||
| RootfsSize: rootfsHeader.Metadata.Size, | ||
| ChainBuildIds: make([]string, 0, len(chain)), | ||
| MemfileLogicalBytes: make([]uint64, 0, len(chain)), | ||
| MemfileMappingCounts: make([]uint32, 0, len(chain)), | ||
| RootfsLogicalBytes: make([]uint64, 0, len(chain)), | ||
| RootfsMappingCounts: make([]uint32, 0, len(chain)), | ||
| } | ||
| for _, c := range chain { | ||
| res.ChainBuildIds = append(res.ChainBuildIds, c.buildID.String()) | ||
| res.MemfileLogicalBytes = append(res.MemfileLogicalBytes, c.memfileBytes) | ||
| res.MemfileMappingCounts = append(res.MemfileMappingCounts, c.memfileMappingCount) | ||
| res.RootfsLogicalBytes = append(res.RootfsLogicalBytes, c.rootfsBytes) | ||
| res.RootfsMappingCounts = append(res.RootfsMappingCounts, c.rootfsMappingCount) | ||
| } | ||
|
|
||
| return res | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a template was loaded from storage and only some chunks have been read,
item.Value().FileSize(ctx)is the local cache file size, not the uncompressed diff size, so copying it into*TotalBytesmakesListCachedBuildsreport the diff as 100% cached. In that partial-cache scenario, schedulers using these new fields will overestimate locality and route work to a node that still has to fetch most of the data remotely.Useful? React with 👍 / 👎.