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

Enable proxy get in multibag environment #1167

Closed
wants to merge 14 commits into from

Conversation

shino
Copy link
Contributor

@shino shino commented Jun 17, 2015

Related PR: basho/riak_cs_multibag#25

  • Refactor manifest record creation
  • Refactor cluster_id retrieval
  • Add utility functions for riak_test

This PR includes two logically unrelated commits with proxy_get functionality

@shino
Copy link
Contributor Author

shino commented Jun 18, 2015

Build is still failing due to deps mismatch. Once the PR for riak_cs_multibag is merged, deps should be updated.

@shino
Copy link
Contributor Author

shino commented Jun 18, 2015

Anyway, this is ready for review.

{mapping, "proxy_get", "riak_cs.proxy_get", [
{default, off},
{datatype, flag},
hidden
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this would be better if this is not hidden, as a guidance for repl user.

@kuenishi
Copy link
Contributor

Unittest failed:

{'EXIT',
    {undef,
        [{riak_cs_lfs_utils,new_manifest,
             [<<"bucket">>,"test_file",
              <<"c2dfd963-49ff-40ce-ba4f-a6e6f3e73c70">>,10485760,<<"ctype">>,
              "md5",
              {dict,0,16,16,8,80,48,
                  {[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]},
                  {{[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]}}},
              1048576,
              {acl_v2,
                  {"tester","tester_id","tester_key_id"},
                  [{{"tester","tester_id"},['FULL_CONTROL']}],
                  {1434,702097,286801}}],
             []},
         {riak_cs_delete_deadlock,'-prop_delete_deadlock/0-fun-0-',1,
             [{file,"test/riak_cs_delete_deadlock.erl"},{line,75}]}]}}
After 1 tests.
{<<"c2dfd963-49ff-40ce-ba4f-a6e6f3e73c70">>,
 [{part_manifest_v1,<<"part_bucket">>,<<"part_key">>,
                    {1434,702097,286340},
                    495,<<"60d51418-b758-4d14-a1ab-190d072c0c7d">>,10485760,
                    undefined,1024}]}
Shrinking.(1 times)
Reason: 
{'EXIT',
    {undef,
        [{riak_cs_lfs_utils,new_manifest,
             [<<"bucket">>,"test_file",
              <<"c2dfd963-49ff-40ce-ba4f-a6e6f3e73c70">>,10485760,<<"ctype">>,
              "md5",
              {dict,0,16,16,8,80,48,
                  {[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]},
                  {{[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]}}},
              1048576,
              {acl_v2,
                  {"tester","tester_id","tester_key_id"},
                  [{{"tester","tester_id"},['FULL_CONTROL']}],
                  {1434,702097,294544}}],
             []},
         {riak_cs_delete_deadlock,'-prop_delete_deadlock/0-fun-0-',1,
             [{file,"test/riak_cs_delete_deadlock.erl"},{line,75}]},
         {eqc_lazy_lists,lazy_map,2,
             [{file,"../src/eqc_lazy_lists.erl"},{line,35}]}]}}
{<<"c2dfd963-49ff-40ce-ba4f-a6e6f3e73c70">>,
 [{part_manifest_v1,<<"part_bucket">>,<<"part_key">>,
                    {1434,702097,286340},
                    1,<<"60d51418-b758-4d14-a1ab-190d072c0c7d">>,10485760,
                    undefined,1024}]}
riak_cs_delete_deadlock:52: eqc_test_...*failed*
in function riak_cs_delete_deadlock:'-eqc_test_/0-fun-3-'/1 (test/riak_cs_delete_deadlock.erl, line 52)
**error:{assertEqual_failed,[{module,riak_cs_delete_deadlock},
                     {line,52},
                     {expression,"quickcheck ( numtests ( ? TEST_ITERATIONS , ? QC_OUT ( ( prop_delete_deadlock ( ) ) ) ) )"},
                     {expected,true},
                     {value,false}]}

@shino
Copy link
Contributor Author

shino commented Jun 19, 2015

Addressed comments and pushed.

@kuenishi
Copy link
Contributor

You also have to fix test/riak_cs_lfs_utils_eqc.erl.

@@ -488,6 +462,7 @@ local_get_block_timeout() ->
?TIMEOUT_CONFIG_FUNC(get_index_range_gckeys_timeout).
?TIMEOUT_CONFIG_FUNC(get_index_range_gckeys_call_timeout).
?TIMEOUT_CONFIG_FUNC(get_index_list_multipart_uploads_timeout).
?TIMEOUT_CONFIG_FUNC(cluster_id_timeout).
Copy link
Contributor

Choose a reason for hiding this comment

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

From the implicit naming convention on these timeouts, this would better be get_clusterid_timeout.

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 configuration item is not introduced by this PR. [1] Changing the name will bring in backward incompatibility.

[1] at tag 2.0: https://github.com/basho/riak_cs/blob/2.0/src/riak_cs_config.erl#L256

@kuenishi
Copy link
Contributor

First round maybe finished; basic approach to resolve the target cluster for proxy_get in block_server look nice. But I feel we don't have to include whole manifest in all block servers. Just a bag id is enough I think.

@kuenishi
Copy link
Contributor

Anyway, good work.

@kuenishi
Copy link
Contributor

Looks like riak_test is also failing ...

2015-06-22 14:36:47.367 [error] <0.472.0> CRASH REPORT Process <0.472.0> with 0 neighbours exited with reason: no case clause matching [] in riak_cs_multibag:cluster_id/2 line 59 in gen_fsm:terminate/7 line 622
2015-06-22 14:36:47.367 [error] <0.473.0> gen_fsm <0.473.0> in state waiting_command terminated with reason: no case clause matching [] in riak_cs_multibag:cluster_id/2 line 59
2015-06-22 14:36:47.368 [error] <0.473.0> CRASH REPORT Process <0.473.0> with 0 neighbours exited with reason: no case clause matching [] in riak_cs_multibag:cluster_id/2 line 59 in gen_fsm:terminate/7 line 622
2015-06-22 14:36:47.368 [error] <0.247.0> Supervisor riak_cs_put_fsm_sup had child undefined started with {riak_cs_put_fsm,start_link,undefined} at <0.472.0> exit with reason no case clause matching [] in riak_cs_multibag:cluster_id/2 line 59 in context child_terminated
2015-06-22 14:36:47.370 [error] <0.261.0> Webmachine error at path "/buckets/riak-test-bucket/objects/riak_test_key1" : {error,{exit,{{{case_clause,[]},[{riak_cs_multibag,cluster_id,2,[{file,"src/riak_cs_multibag.erl"},{line,59}]},{riak_cs_put_fsm,prepare,1,[{file,"src/riak_cs_put_fsm.erl"},{line,408}]},{riak_cs_put_fsm,prepare,2,[{file,"src/riak_cs_put_fsm.erl"},{line,194}]},{gen_fsm,handle_msg,7,[{file,"gen_fsm.erl"},{line,505}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,239}]}]},{gen_fsm,sync_send_event,[<0.472.0>,{augment_data,<<150,86,201,185,18,191,108,125,103,170,177,1,207,254,169,42,162,...>>},...]}},...}} in gen_fsm:sync_send_event/3 line 214

I don't understand why this happens... because put_fsm line 408 says

    ClusterID = riak_cs_mb_helper:cluster_id(BagId),

Will dig in more.

Note: I was using wrong commit.

@shino
Copy link
Contributor Author

shino commented Jun 25, 2015

Rebased and open new PR #1171.

@shino shino closed this Jun 25, 2015
@kuenishi kuenishi deleted the feature/multibag-with-proxy-get branch June 25, 2015 07:34
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.

2 participants