KV Hook Fails with badarg if Search is not Enabled

[beerriot]

If you manage to install the Search KV hook when search is disabled, PUTs to that bucket fail with an uninformative badarg:

1> C:set_bucket(<<"mine">>, [{precommit, [{struct,[{<<"mod">>,<<"riak_search_kv_hook">>},{<<"fun">>,<<"precommit">>}]}]}]).
ok
6> C:put(riak_object:new(<<"mine">>, <<"two">>, <<"foobar">>)).
{error,
    {precommit_fail,
        {hook_crashed,
            {riak_search_kv_hook,precommit,error,badarg}}}}

The badarg happens during the schema fetch:

https://github.com/basho/riak_search/blob/master/src/riak_search_config.erl#L69

because the schema_table ETS table is not created when Search is not enabled.

One fix might be to catch this ets:lookup call, but it's probably a better idea to check whether Search is enabled at the start of the hook, and fail there (with a more descriptive reason) instead.

[jonmeredith]

Could we add a bucket fix up that checks the main search enabled rather than every request?

On Nov 30, 2012, at 12:40 PM, Bryan Fink notifications@github.com wrote:

If you manage to install the Search KV hook when search is disabled, PUTs to that bucket fail with an uninformative badarg:

1> C:set_bucket(<<"mine">>, [{precommit, [{struct,[{<<"mod">>,<<"riak_search_kv_hook">>},{<<"fun">>,<<"precommit">>}]}]}]).
ok
6> C:put(riak_object:new(<<"mine">>, <<"two">>, <<"foobar">>)).
{error,
{precommit_fail,
{hook_crashed,
{riak_search_kv_hook,precommit,error,badarg}}}}
The badarg happens during the schema fetch:

https://github.com/basho/riak_search/blob/master/src/riak_search_config.erl#L69

because the schema_table ETS table is not created when Search is not enabled.

One fix might be to catch this ets:lookup call, but it's probably a better idea to check whether Search is enabled at the start of the hook, and fail there (with a more descriptive reason) instead.

—
Reply to this email directly or view it on GitHub.

[jonmeredith]

We're focusing our efforts on our SOLR-based replacement, yokozuna.

[coderoshi]

@jonmeredith thoughts on this? I could see a argument for setting the error message in 2.0 to something like "This should not be set for new solr-based search". People have already tried to setup yokozuna and follow old search commands, setup precommit hooks. Or just let it keep failing and fix in 2.x.x if it pops up on the mailing list.

[jonmeredith]

Decided to wait-and-see if a problem post-2.0 and we can open a new issue.