-
Notifications
You must be signed in to change notification settings - Fork 74
Add support for writeable CAS backed files #164
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: master
Are you sure you want to change the base?
Add support for writeable CAS backed files #164
Conversation
} | ||
return len(p), nil | ||
} | ||
|
||
func (fp *blockDeviceBackedFilePool) NewFile() (filesystem.FileReadWriter, error) { |
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.
What are your thoughts on not adding a NewFileWithReadLayer(), but simply extending the existing NewFile()?
We could provide some global instance of a zeroReaderAt that does what we want.
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.
The latest submission modifies the interface to NewFile(SparseReaderAt, int64)
which accepts nil at the SparseReaderAt position.
pkg/filesystem/read_layer.go
Outdated
// shrunk (or grown) with truncate calls. Reading past the logical size returns | ||
// zeroes, reading at negative offsets or truncating to negative sizes is an | ||
// error. | ||
type ReadLayer interface { |
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.
Hmmm... I'm not 100% happy with this name. I would have expected this to be named something like 'Source' or 'InitialContentsReader'. Alternatively, we could call this a 'TruncatableReaderAt'?
While there, I think that this addition of new interfaces makes FilePool a bit too big to live in pkg/filesystem. Would you be interested in moving this to a new pkg/filesystem/pool? Then it also becomes a bit more self-documenting that this interface 'belongs' to FilePool.
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.
type TruncatableFileReader interface {
filesystem.FileReader
Truncate(size int64) error
}
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.
type contiguousFileReader struct {
io.ReaderAt
length int64
}
var _ filesystem.FileReader = (*foo)(nil)
func (r * contiguousFileReader) Close() error { return nil }
func (r * contiguousFileReader) GetNextRegionOffset(...) {
if off >= int64(len(f.data)) {
return 0, io.EOF
}
switch regionType {
case filesystem.Data:
return off, nil
case filesystem.Hole:
return int64(len(f.data)), nil
default:
panic("Unknown region type")
}
}
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.
Interface is now called TruncatableSparseReaderAt.
pkg/filesystem/read_layer.go
Outdated
// error. | ||
type ReadLayer interface { | ||
io.ReaderAt | ||
Truncate(size int64) error |
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.
I just realized: doesn't this need a GetNextRegionOffset() to make GetNextRegionOffset() against the pool backed file work correctly? Because whether a part of a file is sparse can now no longer be determined in isolation.
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.
GetNextRegionOffset added to the new interface SparseReaderAt and TruncatableSparseReaderAt implements the SparseReaderAt interface.
pkg/filesystem/read_layer.go
Outdated
Truncate(size int64) error | ||
} | ||
|
||
func NewReadLayer(underlying io.ReaderAt, sizeBytes uint64) ReadLayer { |
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.
func NewTruncationTrackingFileReader(filesystem.FileReader) TruncatableFileReader {
...
}
pkg/filesystem/read_layer.go
Outdated
// shrunk (or grown) with truncate calls. Reading past the logical size returns | ||
// zeroes, reading at negative offsets or truncating to negative sizes is an | ||
// error. | ||
type ReadLayer interface { |
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.
type contiguousFileReader struct {
io.ReaderAt
length int64
}
var _ filesystem.FileReader = (*foo)(nil)
func (r * contiguousFileReader) Close() error { return nil }
func (r * contiguousFileReader) GetNextRegionOffset(...) {
if off >= int64(len(f.data)) {
return 0, io.EOF
}
switch regionType {
case filesystem.Data:
return off, nil
case filesystem.Hole:
return int64(len(f.data)), nil
default:
panic("Unknown region type")
}
}
Remove the in_memory and directory_path file pools. These filepools are better to replace with the block_device backed filepool which supports sparse files and behaves better during circumstances with scarce resources.
ef7719d
to
33bdd88
Compare
96d358d
to
6950f86
Compare
Filepool files with backing content allows the use of read only datasources as initial content for a filepool backed file. It is implemented with efficient copy on write semantics which will copy any sector from the backing store upon writes.
6950f86
to
a96e766
Compare
The latest version which properly supports GetNextRegionOffset became a bite more complicated, this is a stacked PR where the intention is for 165 and 166 to be merged ahead of this pr. |
No description provided.