Skip to content

Commit

Permalink
Style fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
avsej committed Jun 23, 2017
1 parent a0d5af3 commit 8b3b33d
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 93 deletions.
24 changes: 12 additions & 12 deletions rfc/0001-rfc-process.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The RFC process is designed to be lightweight, collaborative and effective. Doin

Before creating an RFC, it needs to be clear if an RFC is actually needed. The following describes each situation where an RFC is mandatory:

- SDK behavior or API changes when interacting with the User OR the Server. So every time either the user or the server communication changes, there is a high likelyhood other SDKs have a similar impact. This explicitly includes utility classes
- SDK behavior or API changes when interacting with the User OR the Server. So every time either the user or the server communication changes, there is a high likelihood other SDKs have a similar impact. This explicitly includes utility classes
which are very likely to be needed on other SDKs.
- The RFC process itself needs to be changed.
- Existing inconsistencies that need to be resolved across the board.
Expand All @@ -32,33 +32,33 @@ If a prototype is already ongoing, it makes sense to drive the RFC alongside the

Two examples: if the java client includes a helper class for retry semantics on rxjava observables (to reduce the verbosity in writing it manually), this does not have an equivalent on any other SDK and therefore doesn't need a RFC. If the java client introduces a helper class for LDAP support, this does have an impact on the other SDKs and needs a RFC.

When it is not clear if an RFC needs to be created, a quick email or discussion in the team should make it clear wheter the change has an overall impact or not.
When it is not clear if an RFC needs to be created, a quick email or discussion in the team should make it clear whether the change has an overall impact or not.

**An RFC MUST be signed off by each SDK representative and consensus must be reached. Once the RFC is accepted, each SDK MUST implement it according to the RFC. If, after acceptance issues show up during implementation, the MUST lead to a revision of the RFC which again needs to be signed off by everyone.**

# General Design

1. Look at latest issue to determine the rfc number (4 digits) - the `RFCID` is in the form *`rfcNumber`***-***`rfcShortName`*
2. open an issue titled `RFCID`, short description of the rfc, does the owner think it requires sdk specifics, etc... tag as appropriate (if we decide to put tags in). the issue allows to "reserve" the rfc number prior to any file being added publicly to the repo.
3. create the rfc branch: `drafts/RFCID`
4. create the rfc file as `RFCID.md`in `/rfc` directory
1. Look at latest issue to determine the RFC number (4 digits) - the `RFCID` is in the form *`rfcNumber`***-***`rfcShortName`*
2. open an issue titled `RFCID`, short description of the RFC, does the owner think it requires sdk specifics, etc... tag as appropriate (if we decide to put tags in). the issue allows to "reserve" the RFC number prior to any file being added publicly to the repo.
3. create the RFC branch: `drafts/RFCID`
4. create the RFC file as `RFCID.md`in `/rfc` directory
5. start the discussion by doing a Pull Request to master
6. **phase one** iterates on the global idea + "proof of concept" sdk specifics proposed by the owner
7. *end of this phase should probably be marked by a first informal round of votes?*
8. **phase two** iterates on sdk specifics: each sdk team/representative can then push commits in the branch to improve on his own sdk specifics
9. once everyone has contributed sdk specifics, the RFC must be signed off by a message following this convention:
8. **phase two** iterates on SDK specifics: each SDK team/representative can then push commits in the branch to improve on his own SDK specifics
9. once everyone has contributed SDK specifics, the RFC must be signed off by a message following this convention:

```
rfc is (:+1:|:-1:) for `teamname` *anything after is a vote comment*
```

examples:

```
> rfc is :-1: for `java` - I'm not happy with the java specifics after toying with it
> rfc is :+1: for `python`
```

Once content is :+1: by all teams, merge into master using the following strategy (it retains discussion on intermediate commits, but produces a linear history in master):

- `git merge --squash -e drafts/RFCID`
Expand Down
46 changes: 33 additions & 13 deletions rfc/0002-subdocapi.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ Creates a **LookupInBuilder** object which you can then use method-chaining to p

Creates a **MutateInBuilder** object which you can then use method-chaining to populate with mutation operations and then later execute.

#### Bucket.RetrieveIn(doc_id, paths) -> DocumentFragment [OPTIONAL]
#### Bucket.RetrieveIn(doc_id, paths...) -> DocumentFragment [OPTIONAL]

This is a shorthand alternative to constructing a builder, for allowing simple, idiomatic retrieval of items.

RetrieveIn(docid, path1, path2, path3)

is equivalent to:

LookupIn(docid).Get(path1).Get(path2).Get(path3).Do()
LookupIn(docid).Get(path1).Get(path2).Get(path3).Do()

In some languages `RetrieveIn` may offer significant syntactic simplicity, and is therefore optional

Expand All @@ -111,9 +111,9 @@ Details on how operations should be encoded may be found within the protocol. No

*single command* encoding saves 7 bytes

> NOTE: For the request: `{npath(2)+flag(1)}` in extras for single, vs `{opcode(1),npath(2),flag(1)}` in multi, saving one byte. For the response: `{status(2)+nvalue(4)}` vs `{...implicit}` saving 6 bytes, saving a total of 7 bytes overall.) per operation for lookups and 5-12 bytes
> NOTE: For the request: `{npath(2)+flag(1)}` in extras for single, vs `{opcode(1),npath(2),flag(1)}` in multi, saving one byte. For the response: `{status(2)+nvalue(4)}` vs `{...implicit...}` saving 6 bytes, saving a total of 7 bytes overall.) per operation for lookups and 5-12 bytes
> NOTE: For the request: `{npath(2)+flag(1)}` in extras for single, vs `{opcode(1),npath(2),flag(1),nvalue(4)}` in multi, saving 5 bytes. For non-counter responses, the payload is empty; for counter responses the response contains `{index(1),status(2),nvalue(4)}`, which is 7 bytes, vs empty (excluding the actual data) for single).) per operation for mutations. Because subdocs primary goal is to save bandwidth, the SDK should ensure to maximize its encoding features.
> NOTE: For the request: `{npath(2)+flag(1)}` in extras for single, vs `{opcode(1),npath(2),flag(1),nvalue(4)}` in multi, saving 5 bytes. For non-counter responses, the payload is empty; for counter responses the response contains `{index(1),status(2),nvalue(4)}`, which is 7 bytes, vs empty (excluding the actual data) for single).) per operation for mutations. Because subdoc`s primary goal is to save bandwidth, the SDK should ensure to maximize its encoding features.
#### Expiration

Expand All @@ -125,7 +125,7 @@ This builder exposes the creation of a set of lookup operations to be performed.

#### L.Get(path) -> L

Retrieves a paths value from a document. Refer to [GET](https://docs.google.com/document/d/1hOQIPMTEFTmdNgWftSZddMtoc57Zi_yuKJ8BW-QJSiQ/edit#heading=h.hfbqcirlfdhy) in the spec. In Memcached this is `PROTOCOL_BINARY_CMD_SUBDOC_GET`
Retrieves a path's value from a document. Refer to [GET](https://docs.google.com/document/d/1hOQIPMTEFTmdNgWftSZddMtoc57Zi_yuKJ8BW-QJSiQ/edit#heading=h.hfbqcirlfdhy) in the spec. In Memcached this is `PROTOCOL_BINARY_CMD_SUBDOC_GET`

#### L.Exists(path) -> L

Expand Down Expand Up @@ -197,11 +197,11 @@ The list above just gives some ideas and should not be thought of as limiting th

In python this can be expressed as:

Assuming the existing path contains: `["Hello", World, null]`
Assuming the existing path contains: `["Hello", "World", null]`

* `array_append(path, 1, 2, 3, 4)` => results in `["Hello", World, null, 1, 2, 3, 4]`
* `array_append(path, 1, 2, 3, 4)` => results in `["Hello", "World", null, 1, 2, 3, 4]`

* `array_append(path, [1,2,3,4])` => results in `["Hello", World, [1,2,3,4]]`
* `array_append(path, [1,2,3,4])` => results in `["Hello", "World", [1,2,3,4]]`

### DocumentFragment

Expand Down Expand Up @@ -291,7 +291,7 @@ These errors are returned by Memcached and should not be exposed to users. They

* `EINVAL` (0x04). This is received if the command was not properly encoded. Notably this also includes specifying an empty path for commands which do not allow it.

* `SUBDOC_INVALID_COMBO` (0xCB). This is received if an invalid opcode was specified in a multi operation. This usually means that the SDK didnt do proper input validation to ensure that mutation and lookup commands werent mixed in the same builder.
* `SUBDOC_INVALID_COMBO` (0xCB). This is received if an invalid opcode was specified in a multi operation. This usually means that the SDK didn't do proper input validation to ensure that mutation and lookup commands weren't mixed in the same builder.

### Error Propagation

Expand All @@ -318,22 +318,43 @@ The C SDK will not use these verbs, and will instead model the subdoc operations
### Go

```code golang
// -- MUTATE IN EXAMPLE --res, err := bucket.MutateIn('testjson', 0, 0). Replace('invalid', 'Frederick'). Do()//err: Subdocument mutation 0 failed (Sub-document path does not exist)//res==nil: true// -- LOOKUP IN EXAMPLE --res, err := bucket.LookupIn('testjson'). Get('name'). Get('invalid'). Do()//err: Could not execute one or more multi lookups or mutations.//res==nil: falseerr = res.Content('name', &value)//err: <nil>//value: Frederickerr = res.Content('invalid', &value)//err: Sub-document path does not exist//value:
// -- MUTATE IN EXAMPLE --
res, err := bucket.MutateIn('testjson', 0, 0).
Replace('invalid', 'Frederick').
Do()
//err: Subdocument mutation 0 failed (Sub-document path does not exist)
//res==nil: true
// -- LOOKUP IN EXAMPLE --
res, err := bucket.LookupIn('testjson').
Get('name').
Get('invalid').
Do()
//err: Could not execute one or more multi lookups or mutations.
//res==nil: false
err = res.Content('name', &value)
//err: <nil>
//value: Frederick
err = res.Content('invalid', &value)
//err: Sub-document path does not exist
//value:
```

### Java

* The *Do()* method for execution of builders will be implemented in Java as *execute()*.

* The *Bucket.retrieveIn()* method wont be implemented in Java, as it doesnt offer that much value with Java verbosity. If users keep wondering why it is not there, then well come back to it.
* The *Bucket.retrieveIn()* method won't be implemented in Java, as it doesn't offer that much value with Java verbosity. If users keep wondering why it is not there, then we'll come back to it.

### .NET

* The *Do()* method for execution of builders will be implemented in .NET as Execute()

### Python

* Does not use a "builder pattern" but rather a list of command specs which are constructed in a similar fashion.
* Does not use a "builder pattern" but rather a list of "command specs" which are constructed in a similar fashion.

# **Unresolved Questions**

Expand Down Expand Up @@ -404,4 +425,3 @@ If signed off, each representative agrees both the API and the behavior will be
* Specify that either *Do* or *Execute* actually sends the commands to the server

* Clarify common semantics with error handling: Both single and multi results should always return a top-level MULTI_FAILURE (or equivalent) error code.

4 changes: 2 additions & 2 deletions rfc/0005-vbucket-retries.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ This will result in an operation following these steps:
1. Go to step 6

### Why 100ms linear retry?
The reason 100ms was chosen was because, it seemed conservative enough that it would definitely throttle the client talking to the network, so we dont end up in a pathological situation. But it is aggressive enough that a normal application probably would not notice the latency from a retry. The reason behind not making it exponential, because of the rate of new incoming requests, if it is on a per-request basis, incoming operations could cause a pathological state where incoming requests continue to build on the load already causing the server to fail requests.
The reason 100ms was chosen was because, it seemed conservative enough that it would definitely throttle the client talking to the network, so we don't end up in a pathological situation. But it is aggressive enough that a normal application probably would not notice the latency from a retry. The reason behind not making it exponential, because of the rate of new incoming requests, if it is on a per-request basis, incoming operations could cause a pathological state where incoming requests continue to build on the load already causing the server to fail requests.

## Errors
No errors will be changed by this proposal. NMV replies MUST be caught and handled within the SDKs (never leaked to users).
Expand All @@ -54,4 +54,4 @@ None.
| .NET | Jeff M.| May 6th, 2016|
| NodeJS | Brett L.| May 6th, 2016 |
| Python | Mark N.| May 6th, 2016 |
| PHP | Sergey A. | May 19th, 2016 |
| PHP | Sergey A. | May 19th, 2016 |
13 changes: 6 additions & 7 deletions rfc/0008-datastructures.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Functionality may be divided into two categories:
* Discreet functionality. This is for languages which do not have generic interfaces to implement. In such
cases, a `List` is a concept which can be manipulated using various discreet functions such as
`list_append`, `list_prepend` and so on. Examples of such languages are Go and PHP.

The following sections include functionality applicable to one or both of the above categories. See below
(in API reference) for required functionality.

Expand Down Expand Up @@ -271,7 +271,7 @@ Handle as normal (i.e. propagate). This would likely result from user interventi
#### Path Mismatch

- If the document is modified outside of datastructures (e.g. used to be a list and is now a dictionary),
- If a wrong input type was supplied
- If a wrong input type was supplied

This will happen if the document is modified outside of datastructures, or if there is a bug within the datastructure implementation, or if the wrong type is supplied (e.g. non-primitive for set addition).

Expand Down Expand Up @@ -323,7 +323,7 @@ while True:
except KeyExistsError:
# Race condition. We'll just do a normal mutate_in in the next iteration
continue

</pre></code></td>
</tr>

Expand Down Expand Up @@ -356,7 +356,7 @@ while True:

<tr>
<td><code>SetAdd(String key, String value, boolean createSet</code></td>

<td><code>M(key).add_unique(key, value)</code>. See <code>MapAdd</code> for
how to handle the <code>create</code> argument.</td>
</tr>
Expand Down Expand Up @@ -417,7 +417,7 @@ Clients may also implement backwards-compatible functionality for features which

### Java

The datastructure implementations will conform to the JDK's Collection API. Each collection will at least need the document's id and a reference to the Bucket object through which to access the subdoc API and in which to store the underlying document.Note the types are for now restricted to JSON simple types (String, boolean, Number, JsonArray, JsonObject).
The datastructure implementations will conform to the JDK's Collection API. Each collection will at least need the document's id and a reference to the Bucket object through which to access the subdoc API and in which to store the underlying document.Note the types are for now restricted to JSON simple types (String, boolean, Number, JsonArray, JsonObject...).

Below are quick examples that showcase this:

Expand Down Expand Up @@ -547,7 +547,7 @@ These ideas were floated while discussing datastructures, but were deemed out of
* **Bitwise operations**
The ability to use integers rather than explicit JSON for boolean values.
this may save storage and bandwidth. _Requires server support_.

* **Large Queues**
Many people require large scalable job queues. The queue currently implemented
in datastructures/subdoc is simply an array with pop/push functionality and is not designed to
Expand All @@ -560,4 +560,3 @@ These ideas were floated while discussing datastructures, but were deemed out of
Currently we only support things like adding and membership checking.
A "proper" set API usually contains things like intersection, union,
and so on.

4 changes: 2 additions & 2 deletions rfc/0010-cbft.md
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,7 @@ An example of the `fragments` *JSON object* is:
```json
"fragments": {
"description": [
"rsey <mark>Beer</mark> Co. is dedicated to crafting quality <mark>beer</mark> for all; from the occasional <mark>beer</mark> drinker to the <mark>beer</mark> aficionado. We believe that good products come from good people, and strive to do our very bes"
"...rsey <mark>Beer</mark> Co. is dedicated to crafting quality <mark>beer</mark> for all; from the occasional <mark>beer</mark> drinker to the <mark>beer</mark> aficionado. We believe that good products come from good people, and strive to do our very bes..."
],
"name": [
"New Jersey <mark>Beer</mark> Company"
Expand All @@ -1281,7 +1281,7 @@ The proposed pseudocode counterpart for the whole `fragments` section is a multi
```java
Map<String, String[]> fragments() { }

String[] field1 = new String[] { "rsey <mark>Beer</mark> Co" };
String[] field1 = new String[] { "...rsey <mark>Beer</mark> Co" };
String[] field2 = new String[] { "New Jersey <mark>Beer</mark> Company" };

Map fragments = new Map();
Expand Down
Loading

0 comments on commit 8b3b33d

Please sign in to comment.