← Back to team overview

openstack team mailing list archive

Re: [Swift] LFS patch (Ia32c9c34)

 

Nexenta's LFS patch (https://review.openstack.org/#/c/7524/) has languished for a while, and I'd like to address that.

First, thank you for your patch submission. This patch adds new functionality that potentially can allow swift to be deployed in more places. The original version of the patch, which you referenced, was quite a bit more complex. Thanks for listening to the feedback from the reviewers and refactoring out most of the complexity. The current state of the patch appears to be much improved. I do hope that the patch can be approved and merged into swift.

However, there are two things which make it more difficult to approve this patch: review time and ability to test.

This patch touches the ring builder, which is a rather complex part of swift. To properly review it, it will take two of the core devs at least a day each. Unfortunately, putting other "normal" job duties on hold for a day or more is very hard to do. This isn't a problem with Nexenta or the patch itself; it actually points to a problem with swift. We shouldn't have a part of the code so integral to the system that requires a full dev day every time it's touched.

The other issue with approving the patch is testing. Any new feature that is merged into swift becomes something that all swift contributors must now support and maintain. The maintenance burden is lessened (but not eliminated) by any sort of testing that can be provided. The LFS patch adds functionality that cannot be well tested. At best, we can only test that the patch doesn't break any existing functionality. But we have no way to ensure that later patches won't break the functionality that this patch provides. Since this patch is currently only really useful with Nexenta's separate lfs middleware for ZFS, and since there is no testing infrastructure set up to test swift on Solaris/ZFS, we cannot offer any sort of support or maintenance for the feature this patch provides.

If Nexenta would like to provide and run some hardware for testing purposes, it would go a long way to helping ensure that this feature and others like it can be properly added to and maintained in swift. If this LFS patch is indeed accepted, it will be Nextenta's responsibility to ensure that all future patches in swift do not break the LFS functionality. (This applies to the previously merged patch for Solaris compatibility, too.)



--John





On Jul 16, 2012, at 3:45 PM, Victor Rodionov wrote:

> Hello
>  
> I've submit patch (https://review.openstack.org/#/c/7101/), that help Swift use special features of file system on that it working.
>  
> One of the  changes in this patch is for reduce number of network replicas of partition if user use self-repairing mirrored device. For this user should add mirror_copies parameter to each device. By default mirror_copies for all devices is 1, so changes of code don't take any effect for current Swift deployments.  For almost all systems three singleton replicas can be replaced by two mirrored replicas. So if all user devices is mirrored (mirror_copies >= 2), then number of network copies of most partition will be reduced, and then for operation like PUT and POST we will make less request. The definition of mirroring specifically requires the local file system detect the bad replica on its own, such as by calculating checksums of the content, and automatically repairing data defects when discovered.  So if one of devices fail recovery will be done by file system without coping data from other device. This changes was made in ring builder and take effect if mirror_copies > 1, so this code is not danger for current Swift users, but for other users can provide new possibility.
>  
> Also this patch add hooks, that can be used for manipulation with file system, when Swift operate with account, container or object files. This hooks used by middleware that is separate project, so if user don't install it this changes will not take effect.
>  
> This feature only enabled by customers that have chosen to install  the enabling software and turn it on and it is easy to test that this patches have no impact on the generic deployments.
>  
> Most of patch code was restructured, most of logic was moved to middleware level and use hooks in Swift code. I create separate project (LFS middleware https://github.com/nexenta/lfs) for now there are only 2 supported file system types (XFS and ZFS) there. Also this middleware provide API for getting file system status information (for example, for ZFS it's current pool status, etc).
>  
> Further the Nexenta side-project is not the only local file system that could provide this form of local replication and data protection.Trading off between network replication and local replication is a valid performance decision. Insisting on a fixed amount of network replication without regard to the degree of local protection provided against data loss would effectively bias the system towards network replication only. Why pay for local data protection if you cannot reduce the amount you pay for network replication? This patch enables solutions that widen the range of choices available to users as to how they protect their data.
>  
> Thanks,
> Victor Rodionov
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp

Attachment: smime.p7s
Description: S/MIME cryptographic signature


Follow ups

References