-
Notifications
You must be signed in to change notification settings - Fork 22
storage: concurrent disk image write. #2395
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 1 commit
999362a
d35f3c5
caf83ac
bfca359
ffc59d8
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 |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import ( | |
| log "github.com/rs/zerolog/log" | ||
| "github.com/threefoldtech/zos/pkg" | ||
| "github.com/threefoldtech/zos/pkg/gridtypes" | ||
| "golang.org/x/sync/errgroup" | ||
| ) | ||
|
|
||
| const ( | ||
|
|
@@ -145,10 +146,35 @@ func (s *Module) DiskWrite(name string, image string) error { | |
| return fmt.Errorf("image size is bigger than disk") | ||
| } | ||
|
|
||
| _, err = io.Copy(file, source) | ||
| if err != nil { | ||
| // writing the image concurrently to speedup the previous sequential write. | ||
| // the sequential write is slow because the data source is from the remote server. | ||
| var ( | ||
| // use errgroup because there is no point in continuing if one of the goroutines failed | ||
| group = new(errgroup.Group) | ||
| concurrentNum int = 5 | ||
xmonader marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| imgSize int64 = imgStat.Size() | ||
| chunkSize = imgSize / int64(concurrentNum) | ||
| ) | ||
|
|
||
| log.Info().Int("concurrentNum", concurrentNum).Msg("writing image concurrently") | ||
| for i := 0; i < concurrentNum; i++ { | ||
| index := i | ||
| group.Go(func() error { | ||
| start := chunkSize * int64(index) | ||
| len := chunkSize | ||
| if index == concurrentNum-1 { //last chunk | ||
| len = imgSize - start | ||
| } | ||
| wr := io.NewOffsetWriter(file, start) | ||
| rd := io.NewSectionReader(source, start, len) | ||
| _, err = io.Copy(wr, rd) | ||
|
||
| return err | ||
| }) | ||
| } | ||
| if err := group.Wait(); err != nil { | ||
| return errors.Wrap(err, "failed to write disk image") | ||
| } | ||
| log.Info().Msg("writing image finished") | ||
|
|
||
| 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.
should be usable on the zero value or with a context if required
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.
could you elaborate more on this?
not possible for now:
io.Copyitself doesn't have context.I think make it context aware will need separate ticket
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.
DiskWritehandled bystoraged, called byprovisiondthroughzbus.So, plain Go context will not really work here.
But of course we can start with
contextinsidestoraged.As a side note, i noticed that
DiskWritekeep going even thoprovisiondalready cancel the deployment/contract.So, we already have real cancellation issue over zbus.
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.
can you please mention the cancellation problem as a known issue on the top of PR to be easily found later on
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.
Maybe create separate issue because it is not really specific to this PR