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.

Sign up for GitHub

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

Bugfix/fd/riak 1937 #529

Merged
merged 4 commits into from
Aug 5, 2015
Merged

Bugfix/fd/riak 1937 #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f68ac3
Exposed accessor for whether yokozuna is enabled, for write_once put …
fadushin Jul 10, 2015
9117cce
Overloaded yz_kv:index to handle binaries, so that we don't have to d…
fadushin Jul 10, 2015
2fab6e6
Added a write_once search test to the yz_pb test suite.
fadushin Jul 27, 2015
eeb90d0
After review, modified yz integration so that we preserve the old yz_…
fadushin Jul 31, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions riak_test/yz_pb.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ confirm() ->
confirm_admin_index(Cluster),
confirm_admin_bad_index_name(Cluster),
confirm_basic_search(Cluster),
confirm_w1c_search(Cluster),
confirm_encoded_search(Cluster),
confirm_search_to_test_max_score_defaults(Cluster),
confirm_multivalued_field(Cluster),
Expand Down Expand Up @@ -82,11 +83,12 @@ create_index(Cluster, BucketType, Index) ->

create_index(Cluster, BucketType, Index, UseDefaultSchema) when
is_boolean(UseDefaultSchema) ->
create_index(Cluster, BucketType, Index, 3, UseDefaultSchema);
create_index(Cluster, BucketType, Index, [{use_default_schema, UseDefaultSchema}]);
create_index(Cluster, BucketType, Index, Nval) when is_integer(Nval) ->
create_index(Cluster, BucketType, Index, Nval, true).

create_index(Cluster, BucketType, Index, Nval, UseDefaultSchema) ->
create_index(Cluster, BucketType, Index, [{n_val, Nval}]);
create_index(Cluster, BucketType, Index, Props) when is_list(Props) ->
Nval = proplists:get_value(n_val, Props, 3),
UseDefaultSchema = proplists:get_value(use_default_schema, Props, true),
Node = select_random(Cluster),
[{Host, Port}] = host_entries(rt:connection_info([Node])),
lager:info("create_index ~s for bucket type ~s [~p]", [Index, BucketType, {Host, Port}]),
Expand All @@ -111,11 +113,14 @@ create_index(Cluster, BucketType, Index, Nval, UseDefaultSchema) ->
?assertEqual([{index, Index}, {schema, SchemaName}, NvalT], IndexData),

%% Add the index to the bucket props
yz_rt:set_bucket_type_index(Node, BucketType, Index, Nval),
yz_rt:set_bucket_type_index(Node, BucketType, Index, [{n_val, Nval} | Props]),
yz_rt:wait_for_bucket_type(Cluster, BucketType),
riakc_pb_socket:stop(Pid),
ok.

create_index(Cluster, BucketType, Index, Nval, UseDefaultSchema) ->
create_index(Cluster, BucketType, Index, [{n_val, Nval}, {use_default_schema, UseDefaultSchema}]).

store_and_search(Cluster, Bucket, Key, Body, Search, Params) ->
store_and_search(Cluster, Bucket, Key, Body, "text/plain", Search, Params).

Expand Down Expand Up @@ -210,6 +215,15 @@ confirm_basic_search(Cluster) ->
Params = [{sort, <<"score desc">>}, {fl, ["*","score"]}],
store_and_search(Cluster, Bucket, "test", Body, <<"text:herp">>, Params).

confirm_w1c_search(Cluster) ->
Index = <<"write_once">>,
Bucket = {Index, <<"b1">>},
create_index(Cluster, Index, Index, [{write_once, true}]),
lager:info("confirm_basic_search ~p", [Bucket]),
Body = "herp derp",
Params = [{sort, <<"score desc">>}, {fl, ["*","score"]}],
store_and_search(Cluster, Bucket, "test", Body, <<"text:herp">>, Params).

confirm_encoded_search(Cluster) ->
Index = <<"encoded">>,
Bucket = {Index, <<"b1">>},
Expand Down
6 changes: 4 additions & 2 deletions riak_test/yz_rt.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,11 @@ set_bucket_type_index(Node, BucketType, Index) ->
lager:info("Set bucket type ~s index to ~s [~p]", [BucketType, Index, Node]),
create_bucket_type(Node, BucketType, [{?YZ_INDEX, Index}]).

set_bucket_type_index(Node, BucketType, Index, NVal) ->
set_bucket_type_index(Node, BucketType, Index, NVal) when is_integer(NVal) ->
set_bucket_type_index(Node, BucketType, Index, [{n_val, NVal}]);
set_bucket_type_index(Node, BucketType, Index, Props) ->
lager:info("Set bucket type ~s index to ~s [~p]", [BucketType, Index, Node]),
create_bucket_type(Node, BucketType, [{?YZ_INDEX, Index},{n_val,NVal}]).
create_bucket_type(Node, BucketType, [{?YZ_INDEX, Index} | Props]).

solr_http({_Node, ConnInfo}) ->
solr_http(ConnInfo);
Expand Down
50 changes: 37 additions & 13 deletions src/yz_kv.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,35 +190,53 @@ has_indexes(RemoteNode) ->
_ -> false
end.


%% @doc Index the data supplied in the Riak Object.
%% The Riak Object should be a serialized object (a binary,
%% which has been serialized using riak_object:to_binary/1)
-spec index_binary(bucket(), key(), binary(), write_reason(), p()) -> ok.
index_binary(Bucket, Key, Obj, Reason, P) ->
index_internal(
Bucket, Key, Obj, Reason, P
).

%% @doc Index the data supplied in the Riak Object.
-spec index(riak_object:riak_object(), write_reason(), p()) -> ok.
index(Obj, Reason, P) ->
index_internal(
riak_object:bucket(Obj), riak_object:key(Obj), Obj, Reason, P
).

%% @private
index_internal(Bucket, Key, Obj, Reason, P) ->
case yokozuna:is_enabled(index) andalso ?YZ_ENABLED of
true ->
Ring = yz_misc:get_ring(transformed),
Copy link
Contributor

Choose a reason for hiding this comment

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

After looking through the code in a walkthrough (esp. the pars the PR doesn't show, haha), @fadushin and I realized that the should_index check should be called before converting the object from binary. This led to a deeper look into some code duplication (around BKey creating, etc...) and whatnot that is happening; so, he's going to be refactoring his changes a bit and just adding a separate index/5 function for binary objects.

case is_owner_or_future_owner(P, node(), Ring) of
true ->
T1 = os:timestamp(),
BKey = {riak_object:bucket(Obj), riak_object:key(Obj)},
BKey = {Bucket, Key},
try
Copy link
Contributor

Choose a reason for hiding this comment

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

@fadushin thinking we can create a helper to avoid the duplication in index/3 and index/5, no?

Index = get_index(BKey),
ShortPL = riak_kv_util:get_index_n(BKey),
case should_index(Index) of
true ->
index(Obj, Reason, Ring, P, BKey, ShortPL, Index);
index(robj(Bucket, Key, Obj), Reason, Ring, P, BKey, ShortPL, Index);
false ->
dont_index(Obj, Reason, P, BKey, ShortPL)
dont_index(robj(Bucket, Key, Obj), Reason, P, BKey, ShortPL)
end,
yz_stat:index_end(?YZ_TIME_ELAPSED(T1))
catch _:Err ->
yz_stat:index_fail(),
Trace = erlang:get_stacktrace(),
case Reason of
delete ->
?ERROR("failed to delete docid ~p with error ~p because ~p",
[BKey, Err, Trace]);
_ ->
?ERROR("failed to index object ~p with error ~p because ~p",
[BKey, Err, Trace])
end
yz_stat:index_fail(),
Trace = erlang:get_stacktrace(),
case Reason of
delete ->
?ERROR("failed to delete docid ~p with error ~p because ~p",
[BKey, Err, Trace]);
_ ->
?ERROR("failed to index object ~p with error ~p because ~p",
[BKey, Err, Trace])
end
end;
false ->
ok
Expand All @@ -227,6 +245,12 @@ index(Obj, Reason, P) ->
ok
end.

%% @private
robj(Bucket, Key, Obj) when is_binary(Obj) ->
riak_object:from_binary(Bucket, Key, Obj);
robj(_Bucket, _Key, Obj) ->
Obj.

%% @private
%%
%% @doc Update the hashtree so that AAE works but don't create any
Expand Down
2 changes: 1 addition & 1 deletion test/yz_component_tests.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

disable_index_test()->
yokozuna:disable(index),
?assertEqual(yz_kv:index({},delete,{}), ok).
?assertEqual(yz_kv:index(riak_object:new({<<"type">>, <<"bucket">>}, <<"key">>, <<"value">>), delete, {}), ok).

disable_search_test()->
yokozuna:disable(search),
Expand Down