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

Fast user get option [JIRA: RCS-240] #1191

Merged
merged 4 commits into from
Jul 29, 2015
Merged

Fast user get option [JIRA: RCS-240] #1191

merged 4 commits into from
Jul 29, 2015

Conversation

kuenishi
Copy link
Contributor

If fast_user_get is turned on, all requests to get user record skip PR=3 get to Riak to avoid unnecessary unavailability. The case this merits is where one of Riak nodes is down slow or going to down, get_fsm should wait for all of them or request fails. For consistency PW=3 when updating user record would be enough. fast_user_get is off by default now, but will be on by default in some future.

All riak_test items have passed with this option (except stats_test which is currently failing continously), but I don't think additional riak_test for this because current riak_test does not simulate any failure (which we have to do some).

@kuenishi kuenishi added this to the 2.1.0 milestone Jul 16, 2015
@Basho-JIRA Basho-JIRA changed the title Add fast user get option Fast user get option [JIRA: RCS-240] Jul 16, 2015
@shino
Copy link
Contributor

shino commented Jul 16, 2015

Nice PR! I agree this is great direction for latency improvements.

I give big 👍 for the simple(st) approach of current diff is sufficient for some users like private object storage purpose. Maybe not in the scope of this PR, for more general purpose or finally to make this fast path by default setting, I want to discuss on pruning of bucket list [1] and consistency.

Pruning path along with fast path can be achieved by using the strong option to get user objects in some of APIs. To avoid latency issue, such APIs should be not hot APIs. The tentative list is:

  • PUT Bucket S3 API and
  • DELETE Bucket S3 API.

For consistency, it would be safer to use strong option for user modifying API calls. These includes following ones as well as above two S3 APIs.

  • POST User admin API (create user)
  • PUT User admin API (modify user)

It's no problem if these discussion will not be reflected in this PR :) I will just create new issue or PR based on this comment. Aside from it, any comments/correction/etc are appreciated.


[1] Short description of pruning deleted buckets

skip this if you already know about it :)

There is a bucket list in user records and deleted buckets will be there as a kind of tombstone. These tombstones should be removed in any way eventually in general purpose, e.g. public object storage, to keep the list bounded. The pruned object will be overwritten only when user objects has been fetched by the strong option.

@kuenishi
Copy link
Contributor Author

Those thoughts are definitely needed when we make this option on by default. However, I thought such APIs finally go to Stanchion and PR=3 and PW=3 saves consistency ... but it does not sound like that ...

In reality, currently most get_user call is done via riak_cs_wm_common , for all API calls. Maybe we have to make the common module API-aware, which is a little bit more hustle. We also need another riak_test injecting node failure by kill or anything. Let's do it after 2.1.0 get out. Will open another issue for that.

@shino
Copy link
Contributor

shino commented Jul 16, 2015

However, I thought such APIs finally go to Stanchion and PR=3 and PW=3 saves consistency ... but it does not sound like that ...

Agreeeeeee. Currently some update for bucket is done by stanchion for bucket objects (and maybe user objects) and done by riak_cs for user objects. This is a problem but totally another issue 🙈

@@ -211,6 +211,12 @@
{datatype, bytesize}
]}.

%% @doc Whether to omit PR=all get at retrieving user info.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is not hidden, warning should be included that pruning never happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What pruning did you mean?

@shino
Copy link
Contributor

shino commented Jul 24, 2015

Will 👍 after discussing on riak-cs.conf documentation and rebasing on current develop.

@kuenishi
Copy link
Contributor Author

Updated.

@kuenishi
Copy link
Contributor Author

Updated.

@@ -30,6 +30,7 @@
is_admin/1,
get_user/2,
get_user_by_index/3,
from_riakc_obj/2,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used from outside?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. This was remain of my effort to try consistency updating user object. Fruitless effort without strong consistency.

@kuenishi
Copy link
Contributor Author

Updated.

borshop added a commit that referenced this pull request Jul 29, 2015
Fast user get option [JIRA: RCS-240]

Reviewed-by: shino
@shino
Copy link
Contributor

shino commented Jul 29, 2015

@borshop merge

@borshop borshop merged commit 9e5d394 into develop Jul 29, 2015
@shino shino deleted the feature/fast-get-user branch July 29, 2015 04:47
@kuenishi
Copy link
Contributor Author

For release note:

Add an option to replace N=all user get option with N=quorum just before authentication. This improves all latency especially in presence of slow (or actually-failing) node, who blocks the whole request flow because of N=all. The price of turning this on is, A user's owned bucket list is never pruned after a bucket is deleted, instead just marked as deleted (RCS-242 / #1193 (RCS-242) (RCS-242) ). This is wrong.

@kuenishi
Copy link
Contributor Author

kuenishi commented Oct 6, 2015

For release note (correct version):

Add an option to replace PR=all user get option with PR=one and R=quorum just before authentication. This improves all latency especially in presence of slow (or actually-failing) node, who blocks the whole request flow because of PR=all. The price of turning this on is, A user's owned bucket list is never pruned after a bucket is deleted, instead just marked as deleted (RCS-242 / #1193 (RCS-242) (RCS-242) (RCS-242) (RCS-242) (RCS-242) (RCS-242) )

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

Successfully merging this pull request may close these issues.

None yet

4 participants