From 48783da4334c3231b1270dafd2f8b4fbb68335dd Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 27 May 2021 20:12:02 +0800 Subject: [PATCH 1/4] proposal: Feature Gates Signed-off-by: Xuanwo --- rfcs/0-feature-gates.md | 117 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 rfcs/0-feature-gates.md diff --git a/rfcs/0-feature-gates.md b/rfcs/0-feature-gates.md new file mode 100644 index 0000000..56c5e6e --- /dev/null +++ b/rfcs/0-feature-gates.md @@ -0,0 +1,117 @@ +--- +author: Xuanwo +status: draft +updated_at: 2021-05-27 +--- + +# Proposal: Feature Gates + +## Background + +Behavior consistence is the most important thing for go-storage. However, we do have different capabilities and limitations in storage services. So the problem comes into how to handle them. + +Our goals are: + +- Behavior consistent by default (invalid operations should be return error) +- Give user abilities to loose the restriction +- Allow user to enable the features they want + +In [GSP-16], we introduced loose mode which is a global flag that controls the behavior when services meet the unsupported pairs. + +- If `loose` is on: This error will be ignored. +- If `loose` is off: Storager returns a compatibility related error. + +However, we removed `loose` in [GSP-20] because we can't figure out [error could be returned too early while in loose mode](https://github.com/beyondstorage/go-storage/issues/233). And loose is so general that it affects nearly all behavior of Storager. + +In [types: Implement pair policy](https://github.com/beyondstorage/go-storage/pull/453), we try to figure out this problem by introducing `PairPolicy`. + +`PairPolicy` controls the behavior of pairs: + +```go +type PairPolicy struct { + All bool + + ... + + // pairs for interface Storager + Create bool + Delete bool + List bool + ListListMode bool + Metadata bool + Read bool + ReadSize bool + ReadOffset bool + ReadIoCallback bool + Stat bool + Write bool + WriteContentType bool + WriteContentMd5 bool + WriteIoCallback bool +} +``` + +If `PairPolicy.Write` is on, Storager will return `services.PairUnsupportedError` while meeting not supported pairs. + +But it doesn't solve the problem: + +- `PairPolicy` is generated in go-storage and can't reflect the capabilities in service. +- `PairPolicy` only used for pair capabilities check, and can't fix the problem introduced in [GSP-86] + +## Proposal + +So I propose to treat loose behavior consistence as a feature, and introduce feature gates in [go-storage]. + +### New Feature: Loose Operation + +By default, [go-storage] will return errors for not supported pairs. If loose operation feature has been enabled, [go-storage] will ignore those errors. + +To enable loose operation, users need to add pairs like: + +```go +s3.WithStorageFeatures(s3.StorageFeatures{ + LooseOpeartionAll: true, + LooseOperationWrite: true, +}) +``` + +> `New` function is special, and we will always enable `loose` for it. + +### New Feature: Virtual Operation + +```go +s3.WithStorageFeatures(s3.StorageFeatures{ + VirtualOperationAll: true, +}) +``` + +### Feature Struct + +[go-storage] will generate feature structs for all services. + +```go +type StorageFeatures struct { + LooseOpeartionAll bool + LooseOperationWrite bool + + VirtualOperationAll bool + VirtualOperationCreateDir bool +} +``` + +## Rationale + + + +## Compatibility + +This proposal will deprecate `PairPolicy`. + +## Implementation + + + +[GSP-16]: ./16-loose-mode.md +[GSP-20]: ./20-remove-loose-mode.md +[GSP-86]: https://github.com/beyondstorage/specs/pull/86 +[go-storage]: https://github.com/beyondstorage/go-storage \ No newline at end of file From ab1c58e3b9271d9c3024126db4a38cb997b626fb Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 27 May 2021 20:13:36 +0800 Subject: [PATCH 2/4] Assign number Signed-off-by: Xuanwo --- rfcs/{0-feature-gates.md => 87-feature-gates.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename rfcs/{0-feature-gates.md => 87-feature-gates.md} (99%) diff --git a/rfcs/0-feature-gates.md b/rfcs/87-feature-gates.md similarity index 99% rename from rfcs/0-feature-gates.md rename to rfcs/87-feature-gates.md index 56c5e6e..e52ec9c 100644 --- a/rfcs/0-feature-gates.md +++ b/rfcs/87-feature-gates.md @@ -4,7 +4,7 @@ status: draft updated_at: 2021-05-27 --- -# Proposal: Feature Gates +# GSP-87: Feature Gates ## Background From 82c9d4ff82213df4143aacf7916bd0497819bd31 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 30 May 2021 12:03:42 +0800 Subject: [PATCH 3/4] Update rfc Signed-off-by: Xuanwo --- rfcs/87-feature-gates.md | 128 +++++++++++++++++++++++++++------------ 1 file changed, 88 insertions(+), 40 deletions(-) diff --git a/rfcs/87-feature-gates.md b/rfcs/87-feature-gates.md index e52ec9c..4b11ef9 100644 --- a/rfcs/87-feature-gates.md +++ b/rfcs/87-feature-gates.md @@ -8,7 +8,7 @@ updated_at: 2021-05-27 ## Background -Behavior consistence is the most important thing for go-storage. However, we do have different capabilities and limitations in storage services. So the problem comes into how to handle them. +Behavior consistency is the most important thing for go-storage. However, we do have different capabilities and limitations in storage services. So the problem comes into how to handle them. Our goals are: @@ -19,7 +19,7 @@ Our goals are: In [GSP-16], we introduced loose mode which is a global flag that controls the behavior when services meet the unsupported pairs. - If `loose` is on: This error will be ignored. -- If `loose` is off: Storager returns a compatibility related error. +- If `loose` is off: Storager returns a compatibility-related error. However, we removed `loose` in [GSP-20] because we can't figure out [error could be returned too early while in loose mode](https://github.com/beyondstorage/go-storage/issues/233). And loose is so general that it affects nearly all behavior of Storager. @@ -29,25 +29,25 @@ In [types: Implement pair policy](https://github.com/beyondstorage/go-storage/pu ```go type PairPolicy struct { - All bool - - ... - - // pairs for interface Storager - Create bool - Delete bool - List bool - ListListMode bool - Metadata bool - Read bool - ReadSize bool - ReadOffset bool - ReadIoCallback bool - Stat bool - Write bool - WriteContentType bool - WriteContentMd5 bool - WriteIoCallback bool + All bool + + ... + + // pairs for interface Storager + Create bool + Delete bool + List bool + ListListMode bool + Metadata bool + Read bool + ReadSize bool + ReadOffset bool + ReadIoCallback bool + Stat bool + Write bool + WriteContentType bool + WriteContentMd5 bool + WriteIoCallback bool } ``` @@ -60,43 +60,91 @@ But it doesn't solve the problem: ## Proposal -So I propose to treat loose behavior consistence as a feature, and introduce feature gates in [go-storage]. +So I propose to treat loose behavior consistency as a feature and introduce feature gates in [go-storage]. + +### Features + +`Feature` means userland optional abilities provided by [go-storage]. + +`Copier` is not a `Feature`. + +It's decided by service providers, the user can't enable `Copier` for a service. + +`LooseOperation` is a `Feature`. + +It's decided by the user, and service providers can't affect its behavior. + +Every `Feature` SHOULD be introduced via [go-storage] RFC process. + +[go-storage] will generate feature gates struct for service: + +```go +type StorageFeatures struct { + LooseOpeartionAll bool + LooseOperationWrite bool + + VirtualOperationAll bool + VirtualOperationCreateDir bool +} + +func WithStorageFeatures(v StorageFeatures) Pair { + return Pair{ + Key: pairStorageFeatures, + Value: v, + } +} +``` + +User can use `WithStorageFeatures` or `WithStorageFeatures` to enable features they want while `NewServicer` or `NewStorager`. Those features CANNOT be changed during runtime. ### New Feature: Loose Operation +We will format `PairPolicy` as a new feature called: `Loose Operation`. + By default, [go-storage] will return errors for not supported pairs. If loose operation feature has been enabled, [go-storage] will ignore those errors. To enable loose operation, users need to add pairs like: ```go s3.WithStorageFeatures(s3.StorageFeatures{ - LooseOpeartionAll: true, - LooseOperationWrite: true, + LooseOpeartionAll: true, + LooseOperationWrite: true, }) ``` > `New` function is special, and we will always enable `loose` for it. -### New Feature: Virtual Operation +### New Feature: Virtual Operation and Virtual Pair -```go -s3.WithStorageFeatures(s3.StorageFeatures{ - VirtualOperationAll: true, -}) -``` +We will introduce a new idea `Virtual` to represents the following state: A service doesn't support some feature, but we can simulate it by some methods. + + +For example: + +- We can have `Virtual Operation` + - S3 doesn't have native support for `CreateDir`, but we can put an Object end with `/` to simulate it. + - S3 doesn't have native support for `Link`, but we can store the link target in object metadata to simulate it. +- We can have `Virtual Pair` + - fs doesn't support `content_md5` for `write`, but we can store it in the file's xattr to simulate it. -### Feature Struct +Those features will affect users data in some way: -[go-storage] will generate feature structs for all services. +- User couldn't read those data without go-storage +- Those data could be changed by other API and go-storage can't detect them. + +So the end-user has to decide whether enable those virtual features or not. + +- If they are enabled, our services will run in the `virtual` mode, likes your virtual machine. And they have to afford the side effects. +- If they don't, our services will behave like they don't implement this feature. + +The virtual feature will give us more power so that we can implement `Link` / `Rename` even when our service doesn't have native support. + +To enable virtual operation, users need to add pairs like: ```go -type StorageFeatures struct { - LooseOpeartionAll bool - LooseOperationWrite bool - - VirtualOperationAll bool - VirtualOperationCreateDir bool -} +s3.WithStorageFeatures(s3.StorageFeatures{ + VirtualOperationAll: true, +}) ``` ## Rationale @@ -114,4 +162,4 @@ This proposal will deprecate `PairPolicy`. [GSP-16]: ./16-loose-mode.md [GSP-20]: ./20-remove-loose-mode.md [GSP-86]: https://github.com/beyondstorage/specs/pull/86 -[go-storage]: https://github.com/beyondstorage/go-storage \ No newline at end of file +[go-storage]: https://github.com/beyondstorage/go-storage From bad480b54ee176022c34a8eea915fddda9888568 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 30 May 2021 12:09:53 +0800 Subject: [PATCH 4/4] Update rfc Signed-off-by: Xuanwo --- rfcs/87-feature-gates.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/rfcs/87-feature-gates.md b/rfcs/87-feature-gates.md index 4b11ef9..7569868 100644 --- a/rfcs/87-feature-gates.md +++ b/rfcs/87-feature-gates.md @@ -1,7 +1,7 @@ --- author: Xuanwo status: draft -updated_at: 2021-05-27 +updated_at: 2021-05-30 --- # GSP-87: Feature Gates @@ -149,7 +149,7 @@ s3.WithStorageFeatures(s3.StorageFeatures{ ## Rationale - +N/A ## Compatibility @@ -157,7 +157,17 @@ This proposal will deprecate `PairPolicy`. ## Implementation - +To implement virtual operation and virtual pair support, we will add new fields into `service.toml`. + +```toml +[namespace.storage.op.write] +simulated = true +optional = ["content_type", "io_callback", "storage_class"] +virtual = ["content_md5"] +``` + +- `simulated`: Mark this operation as a `virtual operation`. +- `virtual`: The list of `virtual pairs`. [GSP-16]: ./16-loose-mode.md [GSP-20]: ./20-remove-loose-mode.md