Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Remove JS extractor support #80

Merged
merged 1 commit into from over 2 years ago

2 participants

Ryan Zezeski Kelly McLaughlin
Ryan Zezeski
Collaborator

bz1144

It's been decided that JavaScript extractor support will be removed.
The main reasons for doing so are:

  1. This feature has never worked so no existing users are being broken.

  2. The documentation around this feature isn't entirely clear and the
    code to support it is ugly, IMO.

  3. Like any other JS support in Riak it has to pay the penalty of
    shipping bytes between the Erlang and JS VMs. There is also potential
    contention for JS VMs depending on the size of the pool and load on
    the system.

  4. Finally, the main reason I can think of for anyone to use the JS
    extractor support is to pull apart incoming JSON objects. This is
    already handled via an Erlang extractor that is invoked any time it
    sees an application/json content-type header. This will be much
    more efficient than using a JS extractor.

Ryan Zezeski
Collaborator

While doing this I checked to make sure our documented ways of setting the extractor worked and it turns out that {qfun, Fun} causes the ring manager to crash. For now I simply removed that method from the documentation but I'd certainly be willing to talk about fixing it/removing it and adding another PR.

Kelly McLaughlin
Collaborator

I get an error from make test:

src/riak_search_kv_hook.erl:326: Warning: function maybe_start_app/1 is unused
make: *** [test] Error 1

Kelly McLaughlin
Collaborator

In regards to the {qfun, Fun} problem, since it's been previously documented and released, I'd say make a card to fix it unless you really feel strongly that it should just be removed. Nice catch btw.

Kelly McLaughlin
Collaborator

+1 to merge.

Ryan Zezeski Remove JS extractor support
bz1144

It's been decided that JavaScript extractor support will be removed.
The main reasons for doing so are:

1. This feature has never worked so no existing users are being broken.

2. The documentation around this feature isn't entirely clear and the
code to support it is ugly, IMO.

3. Like any other JS support in Riak it has to pay the penalty of
shipping bytes between the Erlang and JS VMs.  There is also potential
contention for JS VMs depending on the size of the pool and load on
the system.

4. Finally, the main reason I can think of for anyone to use the JS
extractor support is to pull apart incoming JSON objects.  This is
already handled via an Erlang extractor that is invoked any time it
sees an `application/json` content-type header.  This will be much
more efficient than using a JS extractor.
6ce074c
Ryan Zezeski rzezeski merged commit 9912613 into from August 19, 2011
Ryan Zezeski rzezeski closed this August 19, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Aug 19, 2011
Ryan Zezeski Remove JS extractor support
bz1144

It's been decided that JavaScript extractor support will be removed.
The main reasons for doing so are:

1. This feature has never worked so no existing users are being broken.

2. The documentation around this feature isn't entirely clear and the
code to support it is ugly, IMO.

3. Like any other JS support in Riak it has to pay the penalty of
shipping bytes between the Erlang and JS VMs.  There is also potential
contention for JS VMs depending on the size of the pool and load on
the system.

4. Finally, the main reason I can think of for anyone to use the JS
extractor support is to pull apart incoming JSON objects.  This is
already handled via an Erlang extractor that is invoked any time it
sees an `application/json` content-type header.  This will be much
more efficient than using a JS extractor.
6ce074c
This page is out of date. Refresh to see the latest.
1  include/riak_search.hrl
@@ -8,7 +8,6 @@
8 8
 -define(DEFAULT_INDEX, <<"search">>).
9 9
 -define(RESULTVEC_SIZE, 1000).
10 10
 -define(OPKEY(Tag, Op), {Tag, element(2, Op)}).
11  
--define(JSPOOL_SEARCH_EXTRACT, riak_search_js_extract).
12 11
 
13 12
 -record(search_state, {
14 13
           parent=undefined,
156  src/riak_search_kv_hook.erl
@@ -21,21 +21,13 @@
21 21
 -define(DEFAULT_ARGS,      undefined).
22 22
 
23 23
 -type user_funterm() :: {modfun, user_modname(), user_funname()} |
24  
-                        {qfun, extract_qfun()} |
25  
-                        {jsanon, user_strorbin()} |
26  
-                        {jsanon, {user_strorbin(), user_strorbin()}} |
27  
-                        {jsfun, user_strorbin()}.
  24
+                        {qfun, extract_qfun()}.
28 25
 -type user_modname() :: string() | module().
29 26
 -type user_funname() :: string() | atom().
30  
--type user_strorbin() :: string() | binary().                       
31  
-
32 27
 
33 28
 -type extractdef() :: {funterm(), args()}.
34 29
 -type funterm() :: {modfun, atom(), atom()} |
35  
-                   {qfun, extract_qfun()} |
36  
-                   {jsanon, binary()} |
37  
-                   {jsanon, {binary(), binary()}} |
38  
-                   {jsfun, binary()}.
  30
+                   {qfun, extract_qfun()}.
39 31
 
40 32
 -type riak_object() :: tuple(). % no good way to define riak_object
41 33
 
@@ -130,12 +122,6 @@ validate_funterm({modfun, Mod, Fun}) ->
130 122
     {modfun, to_modfun(Mod), to_modfun(Fun)};
131 123
 validate_funterm({qfun, Fun}=FunTerm) when is_function(Fun) ->
132 124
     FunTerm;
133  
-validate_funterm({jsanon, {Bucket, Key}}) ->
134  
-    {jsanon, {to_binary(Bucket), to_binary(Key)}};
135  
-validate_funterm({jsanon, Source}) ->
136  
-    {jsanon, to_binary(Source)};
137  
-validate_funterm({jsfun, Name}) ->
138  
-    {jsfun, to_binary(Name)};
139 125
 validate_funterm(FunTerm) ->
140 126
     throw({"cannot parse funterm", FunTerm}).
141 127
 
@@ -145,33 +131,8 @@ erlify_json_funterm(<<"erlang">>, Props) ->
145 131
     Fun = to_modfun(proplists:get_value(<<"function">>, Props, undefined)),
146 132
     Arg = proplists:get_value(<<"arg">>, Props, undefined),
147 133
     {{modfun, Mod, Fun}, Arg};
148  
-erlify_json_funterm(<<"javascript">>, Props) ->
149  
-    Source = proplists:get_value(<<"source">>, Props, undefined),
150  
-    Name = proplists:get_value(<<"name">>, Props, undefined),
151  
-    Bucket = proplists:get_value(<<"bucket">>, Props, undefined),
152  
-    Key = proplists:get_value(<<"key">>, Props, undefined),
153  
-    Arg = proplists:get_value(<<"arg">>, Props, undefined),
154  
-    case Source of
155  
-        undefined ->
156  
-            case Name of
157  
-                undefined ->
158  
-                    case (Bucket == undefined) orelse (Key == undefined) of
159  
-                        true ->
160  
-                            throw("javascript kv/search extractor must have"
161  
-                                  "name, source, or bucket and key");
162  
-                        _ ->
163  
-                            {{jsanon, {Bucket, Key}}, Arg}
164  
-                    end;
165  
-                _ ->
166  
-                    {{jsfun, Name}, Arg}
167  
-            end;
168  
-        _ ->
169  
-            {{jsanon, Source}, Arg}
170  
-    end;
171  
-erlify_json_funterm(Lang, _Props) ->
172  
-    throw({"kv/search extractors must be written in erlang or javascript", Lang}).
173  
-
174  
-     
  134
+erlify_json_funterm(_Lang, _Props) ->
  135
+    throw({"Malformed KV/Search extractor", _Props}).
175 136
 
176 137
 -spec to_modfun(list() | atom()) -> atom().
177 138
 to_modfun(List) when is_list(List) ->
@@ -184,16 +145,6 @@ to_modfun(Atom) when is_atom(Atom) ->
184 145
     Atom;
185 146
 to_modfun(Val) ->
186 147
     throw({"cannot convert to module/function name", Val}).
187  
-   
188  
--spec to_binary(atom() | string() | binary()) -> binary().
189  
-to_binary(Atom) when is_atom(Atom) ->
190  
-    atom_to_binary(Atom, utf8);
191  
-to_binary(List) when is_list(List) ->
192  
-    list_to_binary(List);
193  
-to_binary(Bin) when is_binary(Bin) ->
194  
-    Bin;
195  
-to_binary(Val) ->
196  
-    throw({"cannot convert to binary", Val}).
197 148
 
198 149
 %%
199 150
 %% Index the provided riak object and return ok on success.
@@ -252,51 +203,8 @@ run_extract(RiakObject, DefaultField, {{modfun, Mod, Fun}, Arg}) ->
252 203
     Mod:Fun(RiakObject, DefaultField, Arg);
253 204
 run_extract(RiakObject, DefaultField, {{qfun, Fun}, Arg}) ->
254 205
     Fun(RiakObject, DefaultField, Arg);
255  
-run_extract(RiakObject, DefaultField, {{Js, FunTerm}, Arg})
256  
-  when Js == jsanon; Js == jsfun ->
257  
-    Fun = if is_binary(FunTerm) -> FunTerm;
258  
-             is_tuple(FunTerm) ->
259  
-                  {Bucket, Key} = FunTerm,
260  
-                  {ok, Client} = riak:local_client(),
261  
-                  try 
262  
-                      {ok, JSObj} = Client:get(Bucket, Key, 1),
263  
-                      hd(riak_object:get_values(JSObj))
264  
-                  catch
265  
-                      error:{badmatch,{error,notfound}} ->
266  
-                          throw({fail, {"Extractor not found", {Bucket, Key}}})
267  
-                  end
268  
-          end,
269  
-    JsRObj = riak_object:to_json(RiakObject),
270  
-    case Arg of
271  
-        undefined ->
272  
-            JsArg = null;
273  
-        _ ->
274  
-            JsArg = Arg
275  
-    end,
276  
-    case riak_kv_js_manager:blocking_dispatch(?JSPOOL_SEARCH_EXTRACT, {{Js, Fun}, [JsRObj, DefaultField, JsArg]}, 5) of
277  
-        {ok, <<"fail">>} ->
278  
-            throw(fail);
279  
-        {ok, [{<<"fail">>, Message}]} ->
280  
-            throw({fail, Message});
281  
-        {ok, JsonFields} ->
282  
-            erlify_json_extract(JsonFields);
283  
-        {error, Error} ->
284  
-            lager:error("Error executing kv/search hook: ~s", [Error]),
285  
-            throw({fail, Error})
286  
-    end;
287 206
 run_extract(_, _, ExtractDef) ->
288 207
     throw({error, {not_implemented, ExtractDef}}).
289  
-
290  
-erlify_json_extract(R) ->
291  
-    erlify_json_extract(R, []).
292  
-
293  
-erlify_json_extract([], Acc) ->
294  
-    lists:reverse(Acc);
295  
-erlify_json_extract([{FieldName, FieldData} | Rest], Acc) when is_binary(FieldName),
296  
-                                                                  is_binary(FieldData) ->
297  
-    erlify_json_extract(Rest, [{FieldName, FieldData} | Acc]);
298  
-erlify_json_extract(R, _Acc) ->
299  
-    throw({fail, {bad_json_extractor, R}}).
300 208
         
301 209
 %% Get the precommit hook from the bucket and strip any
302 210
 %% existing index hooks.
@@ -396,67 +304,12 @@ run_qfun_extract_test() ->
396 304
     Extractor = validate_extractor({{qfun, Fun1}, undefined}),
397 305
     ?assertEqual([{<<"data">>,<<"some data">>}],
398 306
                  run_extract(TestObj, <<"data">>, Extractor)).
399  
- 
400  
-    
401  
-
402  
-anon_js_extract_test() ->
403  
-    maybe_start_app(sasl),
404  
-    maybe_start_app(erlang_js),
405  
-    JsSup = maybe_start_link(riak_kv_js_sup:start_link()),
406  
-    JsMgr = maybe_start_link(riak_kv_js_manager:start_link(?JSPOOL_SEARCH_EXTRACT, 2)),
407  
-
408  
-    %% Anonymous JSON function with default argument
409  
-    %% Join together all the values in a search field
410  
-    %% called "data" and the argument as "arg"
411  
-    JustObjectSource = "function(o, d) {
412  
-                var vals = [];
413  
-                for (var i = 0; i < o.values.length; i++) {
414  
-                  vals.push(o.values[i].data);
415  
-                }
416  
-                data = vals.join(\" \");
417  
-                return {\"data\":data};
418  
-              }",
419  
-    ObjectArgSource = "function(o,d,a) {
420  
-                var vals = [];
421  
-                for (var i = 0; i < o.values.length; i++) {
422  
-                  vals.push(o.values[i].data);
423  
-                }
424  
-                data = vals.join(\" \");
425  
-                return {\"data\":data, \"arg\":a.f};
426  
-              }",
427  
- 
428  
-    %% Try the anonymous function
429  
-    O = conflict_test_object(),
430  
-    Extractor1 = validate_extractor({{jsanon, JustObjectSource}, undefined}),
431  
-    ?assertEqual([{<<"data">>,<<"some data">>}],
432  
-                 run_extract(O, <<"data">>, Extractor1)),
433  
-                 
434  
-    %% Anonymous JSON function with provided argument
435  
-    %% Arg = {struct [{<<"f">>,<<"v">>}]},
436  
-    Arg = {struct, [{<<"f">>,<<"v">>}]},
437  
-    Extractor2 = validate_extractor({{jsanon, ObjectArgSource}, Arg}),
438  
-    ?assertEqual([{<<"data">>,<<"some data">>}, 
439  
-                  {<<"arg">>, <<"v">>}],
440  
-                 run_extract(O, <<"value">>, Extractor2)),
441  
-
442  
-    stop_pid(JsMgr),
443  
-    stop_pid(JsSup),
444  
-    application:stop(erlang_js).
445 307
 
446 308
 conflict_test_object() ->
447 309
     O0 = riak_object:new(<<"b">>,<<"k">>,<<"v">>),
448 310
     riak_object:set_contents(O0, [{dict:new(), <<"some">>},
449 311
                                   {dict:new(), <<"data">>}]).
450 312
     
451  
-
452  
-maybe_start_app(App) ->
453  
-    case application:start(App) of
454  
-        {error, {already_started, _}} ->
455  
-            ok;
456  
-        ok ->
457  
-            ok
458  
-    end.
459  
-
460 313
 maybe_start_link({ok, Pid}) -> 
461 314
     Pid;
462 315
 maybe_start_link({error, {already_started, _}}) ->
@@ -469,4 +322,3 @@ stop_pid(Pid) ->
469 322
     exit(Pid, kill).
470 323
 
471 324
 -endif. % TEST
472  
-    
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.