This repository has been archived by the owner on Sep 12, 2018. It is now read-only.
StorageDriver interface changes and cleanup #820
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change brings the storagedriver API in line with the Go standard library's use of int64 for offsets. The main benefit is simplicity in interfacing with the io library reducing the number of type conversions in simple code.
To support single-flight Size and ModTime queries against backend storage file, we are replacing the CurrentSize call with a Stat call. A FileInfo interface is provided for backends to provide a type, with a default implementation called FileInfoInternal, for use by driver implementations. More work needs to follow this change to update all the driver implementations.
We are change the the rpc call for WriteStream to not require the size argument, opting to drive the process with io.Reader. The main issue was that io.Reader may return io.EOF before reaching size, making the error handling around this condition for callers more complex. To complement this, WriteStream now returns the number of successfully written bytes. The method no longer requires an io.ReadCloser, opting to require just an io.Reader. This keeps the reader under the control of the caller, which provides more flexibility. This also begins to address some of the problems described in docker-archive#791.
This change updates the testsuite to migrate to the new driver interface. This includes the new Stat call, changes to int64 over uint64 and the changes to the WriteStream signature. Several test cases have been added to vet implementations against various assumptions.
The filesystem driver has been migrated to impleemnt the storagedriver interface changes. Most interetingly, this provides a filesystem-based implementation of the Stat driver call. With this comes some refactoring of Reads and Write to be much simpler and more robust. The IPC tests have been disabled to stability problems that we'll have to troubleshoot at a later date.
This change started out as simply updating the existing inmemory driver to implement the new Stat call. After struggling with the map based implementation, it has been refactored to be a tree-based implementation. This process has exposed a few missing error cases in the StorageDriver API that should be addressed in the coming weeks.
This change updates the backend storage package that consumes StorageDriver to use the new Stat call, over CurrentSize. It also makes minor updates for using WriteStream and ReadStream.
Several checks for ReadStream with offset around boundary conditions were missing. The new checks ensure negative offsets are detected and io.EOF is returned properly when trying to read past the end of a file. The filesystem and inmemory driver have been updated accordingly. An outline of missing checks for List are also part of this commit. Action will be taken here based on discussion in issue docker-archive#819.
The packages causing build errors are being disabled for now to let us split up the work in the different driver implementations without blocking integration into the main branch. The s3 and azure implementations need some effort to add Stat support. The ipc package needs that work plus some care around hanging send calls.
Exists was returning an error when encountering a PathNotFoundError when it should just return false without an error.
The tests are using way too much memory with the race detector enabled causing the build machines to fall over. Cursory profiling shows no leaks but it may need a closer look. For now, it will be disabled but this cannot be permanent.
stevvooe
force-pushed
the
ng-storagedriver-updates
branch
from
fcfc30e to
e15e07c
Compare
|
LGTM |
dmp42
added a commit
that referenced
this pull request
Dec 8, 2014
StorageDriver interface changes and cleanup
|
LGTM |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mostly, this PR begins the process of hardening the storagedrivers. Several interface changes are included, in addition to cleanup in the filesystem and inmemory implementations. More tests have been added to solidify some ambiguities, as well.
Unfortunately, this PR will break the build. Specifically, we are having issues with ipc and the azure and s3 drivers need to be updated separately. This is necessary for coordination and progress.
This pull request supersedes #716 and addresses issues called out in #791 and #781. #814 will be addressed in a separate PR.
Please see the commit messages for details.
@BrianBland @AndreyKostov @dmp42 @ahmetalpbalkan