Skip to content
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

Notion of transaction for the whole request/response cycle #194

Closed
leplatrem opened this issue Sep 16, 2015 · 3 comments
Closed

Notion of transaction for the whole request/response cycle #194

leplatrem opened this issue Sep 16, 2015 · 3 comments

Comments

@leplatrem
Copy link
Contributor

After discussing with @phrawzty, we should probably expose clearly a limitation of the current implementation.

There is no notion of transaction for the whole cycle request/response.

Which basically means that every interactions with the backends are performed atomically and not rolled back if something wrong happens afterwards, before serving the response.

I believe we can be pretty confident in the python code that is executed between storage interaction and returning the response. However we can't guarantee «per-request integrity» when performing several backend operations.

This mainly applies:

  • /batch requests
  • requests that interact with both storage and permissions backend (eg. requests with both data permissions attributes)
  • edit: Purging tombstones when deleting bucket/collections

It is especially problematic with non idempotent requests, like POST or PUT using ETag headers (404 on delete could still be ignored).

One way of fixing this issue would be:

  • Creating a savepoint when the request kicks in
  • Rollback if the response status is not 2XX
  • Commit otherwise.
  • Rewriting the subquest approach for the batch

Meanwhile, we should use this issue to think about it and find a comforting way to present the currrent limitation in the documentation :)

leplatrem added a commit to mozilla-services/cliquet that referenced this issue Oct 20, 2015
**Rationale*:

This small refactor paves the way for:

* persisting backend connections on current request (instead of pull/push from queue for each operation performed with one request, e.g batched requests)
* having one transaction per request (with a tween to connect / commit). Particularly interesting for batched requests. (see Kinto/kinto#194)
* have one event per request, with every created/updated/deleted records of a batch

Of course, it may seem ugly to pass the request, but it is advocated [in Pyramid docs](http://pyramid.readthedocs.org/en/1.5-branch/narr/threadlocals.html#why-you-shouldn-t-abuse-thread-locals):

> get_current_request function should never be called because it’s “easier” or “more elegant” to think about calling it **than to pass a request through a series of function calls when creating some API design**. Your application should instead almost certainly pass data derived from the request around rather than relying on being able to call this function to obtain the request in places that actually have no business knowing about it.

We could pass a derived info like `request.bound_data`, but I like the fact to pass the request object.

Especially because it gives a lot of freedom in possible backend implementation (*imagine a box.com storage backend where original request headers are manipulated before being sent to third party service...*)
@tarekziade
Copy link
Contributor

side note: once this is implemented, it would be cool to have transaction-level events (start/success/failure) with a transaction number (uuid?) attached

leplatrem added a commit to mozilla-services/cliquet that referenced this issue Nov 6, 2015
leplatrem added a commit to mozilla-services/cliquet that referenced this issue Nov 12, 2015
@leplatrem
Copy link
Contributor Author

Note: This was landed in cliquet master (2.11.dev).

@leplatrem
Copy link
Contributor Author

Done in 1.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants