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

Add switch to choose listkeys or fold_objects in storage calclation #1089

Merged
merged 4 commits into from
Mar 12, 2015

Conversation

kuenishi
Copy link
Contributor

Rebased version of and supersedes #1051 (RCS-34).

%% when scanning the whole bucket.
-spec use_2i_for_storage_calc() -> boolean().
use_2i_for_storage_calc() ->
riak_cs_list_objects_utils:fold_objects_for_list_keys()
Copy link
Contributor

Choose a reason for hiding this comment

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

This setting item is already deprecated one. Should we care about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The deprecation plan is to mention in 2.0 release notes and remove the code later, like late 2.0.x or 2.1. That's why I still hold this config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this was to ensure Riak is later than 1.4 - having 2i <<"key">> index and return_value=true in vnode folding. CS can know this thanks to riak_pb_csbucket.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, understood.

@shino
Copy link
Contributor

shino commented Mar 11, 2015

Will the new configuration item go into cuttlefish schema?

@kuenishi
Copy link
Contributor Author

The reason why I still keep default value of use_2i_for_storage_calc as false is just I don't want to change many things at once. But I'd like to change default as true until all Riak <= 2.0.5 updated later version, at least. Maybe timing is good at 1.4 deprecation.

@kuenishi
Copy link
Contributor Author

I don't think we have to introduce this configuration in riak-cs.conf.
With the same reason I want to keep this parameter rather well-unknown. If we change default value, then do really operators have to know it? This is minor optimisation, so we'd better omit the responsibility to explain (or even announce loud) what the configuration exactly is. Just enough and worth for advanced operators.

@kuenishi
Copy link
Contributor Author

  • Added very stupid riak_test
  • Get rid of unnecessary configuration (fold_objects_for_list_keys)
  • Bigger upper bound of 2i input

Ready for review again.

@shino
Copy link
Contributor

shino commented Mar 12, 2015

The reason why I still keep default value of use_2i_for_storage_calc as false is just I don't want to change many things at once.

Understood. I won't test use_2i_for_storage_calc=true with Riak 2.0.5- .

@shino
Copy link
Contributor

shino commented Mar 12, 2015

Not disk intensive, but did micro benchmark on my laptop, with riak_ee 2.0 branch and got similar results as you had.

About 830 K keys

> application:set_env(riak_cs, use_2i_for_storage_calc, true).

13:07:46.817 [info] Finished storage calculation in 43 seconds.
13:09:20.953 [info] Finished storage calculation in 42 seconds.
13:10:22.254 [info] Finished storage calculation in 42 seconds.

> application:set_env(riak_cs, use_2i_for_storage_calc, false).

13:12:45.069 [info] Finished storage calculation in 81 seconds.
13:14:04.226 [info] Finished storage calculation in 75 seconds.
13:15:24.673 [info] Finished storage calculation in 78 seconds.

> application:set_env(riak_cs, use_2i_for_storage_calc, true). %% sanity check

13:16:23.934 [info] Finished storage calculation in 41 seconds.

@shino
Copy link
Contributor

shino commented Mar 12, 2015

riak_test passed with riak_ee 2.0 branch.

borshop added a commit that referenced this pull request Mar 12, 2015
…-rebased

Add switch to choose listkeys or fold_objects in storage calclation

Reviewed-by: shino
@shino
Copy link
Contributor

shino commented Mar 12, 2015

@borshop merge

@borshop borshop merged commit 8993421 into develop Mar 12, 2015
@shino shino deleted the feature/storage-calc-optimization-rebased branch March 12, 2015 07:43
@shino
Copy link
Contributor

shino commented Sep 25, 2015

Memo: The change basho/riak_kv#1072 is included in Riak 2.0.6 and 2.1.0+.

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

Successfully merging this pull request may close these issues.

None yet

3 participants