-
Notifications
You must be signed in to change notification settings - Fork 438
Export rootfs in the background during pause()/snapshot() #3320
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
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| //go:build linux | ||
|
|
||
| package block | ||
|
|
||
| import ( | ||
| "os" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/e2b-dev/infra/packages/shared/pkg/storage/header" | ||
| ) | ||
|
|
||
| // TestCacheDiffMetadata_MatchesExportToDiff verifies DiffMetadata (used to build | ||
| // the diff header synchronously) produces the same dirty/empty bitmaps as the | ||
| // metadata ExportToDiff computes while copying, so a background seal's header is | ||
| // exact. | ||
| func TestCacheDiffMetadata_MatchesExportToDiff(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| blockSize := int64(header.PageSize) | ||
| numBlocks := int64(4) | ||
|
|
||
| c, err := NewCache(blockSize*numBlocks, blockSize, t.TempDir()+"/cache", false) | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { _ = c.Close() }) | ||
|
|
||
| // Dirty block 0, zero block 2, leave 1 and 3 untouched. | ||
| dirtyBlock := make([]byte, blockSize) | ||
| for i := range dirtyBlock { | ||
| dirtyBlock[i] = 0xAB | ||
| } | ||
| _, err = c.WriteAt(dirtyBlock, 0) | ||
| require.NoError(t, err) | ||
| _, err = c.WriteZeroesAt(2*blockSize, blockSize) | ||
| require.NoError(t, err) | ||
|
|
||
| // Metadata read without copying. | ||
| meta, err := c.DiffMetadata() | ||
| require.NoError(t, err) | ||
|
|
||
| // Metadata computed by the actual export. | ||
| out, err := os.CreateTemp(t.TempDir(), "diff-*") | ||
| require.NoError(t, err) | ||
| t.Cleanup(func() { _ = out.Close() }) | ||
|
|
||
| exportMeta, err := c.ExportToDiff(t.Context(), out) | ||
| require.NoError(t, err) | ||
|
|
||
| require.True(t, meta.Dirty.Equals(exportMeta.Dirty), "dirty bitmaps must match") | ||
| require.True(t, meta.Empty.Equals(exportMeta.Empty), "empty bitmaps must match") | ||
| require.Equal(t, exportMeta.BlockSize, meta.BlockSize) | ||
| require.EqualValues(t, 1, meta.Dirty.GetCardinality()) | ||
| require.EqualValues(t, 1, meta.Empty.GetCardinality()) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| //go:build linux | ||
|
|
||
| package build | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
|
|
||
| "github.com/e2b-dev/infra/packages/shared/pkg/storage" | ||
| "github.com/e2b-dev/infra/packages/shared/pkg/utils" | ||
| ) | ||
|
|
||
| // ErrDeferredSealFailed marks a deferred rootfs seal that failed. The seal runs | ||
| // exactly once, so the failure is permanent: the promise stays settled with this | ||
| // error and every subsequent data-bearing method on the diff returns it. Callers | ||
| // such as the pause-upload retry loop match on it to stop retrying a diff that | ||
| // can never materialize (rather than burning the whole retry budget on it). | ||
| var ErrDeferredSealFailed = errors.New("deferred rootfs seal failed") | ||
|
|
||
| // deferredDiff is a Diff whose backing data is produced asynchronously. It is | ||
| // returned synchronously from a pause that seals the rootfs in the background: | ||
| // the cache key and block size are known up front (so DiffStore.Add and the | ||
| // upload's compress-config validation work immediately), while every data- | ||
| // bearing method blocks on the inner promise until the background seal resolves | ||
| // the real Diff. | ||
| // | ||
| // The producer MUST always resolve the promise — with the sealed Diff on success | ||
| // or an error on failure — otherwise the data methods (and Close) block forever. | ||
| type deferredDiff struct { | ||
| cacheKey DiffStoreKey | ||
| blockSize int64 | ||
| inner *utils.SetOnce[Diff] | ||
| } | ||
|
|
||
| var _ Diff = (*deferredDiff)(nil) | ||
|
|
||
| // NewDeferredDiff wraps a promise of a Diff. cacheKey and blockSize are the | ||
| // synchronously-known identity of the diff; inner is resolved by the background | ||
| // sealer with the materialized Diff (or an error). | ||
| func NewDeferredDiff(cacheKey DiffStoreKey, blockSize int64, inner *utils.SetOnce[Diff]) Diff { | ||
| return &deferredDiff{ | ||
| cacheKey: cacheKey, | ||
| blockSize: blockSize, | ||
| inner: inner, | ||
| } | ||
| } | ||
|
|
||
| func (d *deferredDiff) CacheKey() DiffStoreKey { | ||
| return d.cacheKey | ||
| } | ||
|
|
||
| func (d *deferredDiff) BlockSize() int64 { | ||
| return d.blockSize | ||
| } | ||
|
|
||
| // sealed reports, without blocking, whether the background seal has resolved the | ||
| // promise (with the materialized diff or an error). Cache eviction uses it to | ||
| // skip a not-yet-sealed diff rather than block on the data methods (which wait | ||
| // on the seal) or evict a fresh, still-in-flight snapshot. | ||
| func (d *deferredDiff) sealed() bool { | ||
| select { | ||
| case <-d.inner.Done: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| func (d *deferredDiff) CachePath(ctx context.Context) (string, error) { | ||
| inner, err := d.inner.WaitWithContext(ctx) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return inner.CachePath(ctx) | ||
| } | ||
|
|
||
| func (d *deferredDiff) ReadAt(ctx context.Context, p []byte, off int64, ft *storage.FrameTable) (int, error) { | ||
| inner, err := d.inner.WaitWithContext(ctx) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
|
|
||
| return inner.ReadAt(ctx, p, off, ft) | ||
| } | ||
|
|
||
| func (d *deferredDiff) Slice(ctx context.Context, off, length int64, ft *storage.FrameTable) ([]byte, error) { | ||
| inner, err := d.inner.WaitWithContext(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return inner.Slice(ctx, off, length, ft) | ||
| } | ||
|
|
||
| func (d *deferredDiff) Size(ctx context.Context) (int64, error) { | ||
| inner, err := d.inner.WaitWithContext(ctx) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
|
|
||
| return inner.Size(ctx) | ||
| } | ||
|
|
||
| func (d *deferredDiff) FileSize(ctx context.Context) (int64, error) { | ||
| inner, err := d.inner.WaitWithContext(ctx) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
|
|
||
| return inner.FileSize(ctx) | ||
| } | ||
|
bchalios marked this conversation as resolved.
|
||
|
|
||
| // Close waits for the seal to resolve and closes the materialized diff. If the | ||
| // seal failed there is nothing to close (the producer cleans up the partial file | ||
| // on error), so only close when the diff actually materialized. | ||
| func (d *deferredDiff) Close() error { | ||
|
Contributor
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 seems to be only called on disk pressure-caused chunk eviction, but isn't on the TTL-caused eviction. Doesn't look material with 25h TTL, but becomes a problem if we choose to shorten TTL for some reason.
Contributor
Author
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. added logic to close an unsealed |
||
| if inner, err := d.inner.Wait(); err == nil { | ||
| return inner.Close() | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
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.
PR summary: we should make it clearer that the latency isn't eliminated in the case of immediate resume - it migrates to the pause.
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.
updated the PR summary.