search

basho / riak_kv

Riak Key/Value Store
Apache License 2.0
650 stars 233 forks source link

Counters throw an unicode error when using js mapreduce #634

Open lenary opened 11 years ago

lenary commented 11 years ago

When doing a js mapreduce over a bucket that contains counter values, you get wonderful unicode errors as it tries to make the counter binary into a string.

For the moment js mapreduce of counters is entirely unsupported, but if someone has counters in a bucket that they also want to js mapreduce over, they're going to have a bad time.

Here's a post on the ML where it's happened (though the guy was explicitly trying to mapreduce counters, the error is the same): http://markmail.org/message/q73gsfvvrkthludo

Perhaps we should have a mechanism where we can include custom serialisation for an object (or sibling) to a value for JS mapreduce, but I'm not sure how easy/efficient that would be.

jtuple commented 10 years ago

Is this still an issue? If so, is this something we are planning to fix before 2.0 final or are we just planning to document the limitation and consider fixing in 2.1? Assigning to 2.0-beta milestone to force us to notice this issue and make a decision.

lenary commented 10 years ago

So the "fix" would be to include custom serialization, which feels like quite a bit of work. My feeling is document that they cause errors, and leave them at that.

I'm imagining that all our data types will cause errors in js mapred as we're compressing them within term_to_binary.

To be clear: there is no issue in Erlang Mapreduce, because erlang can cope, and you can call the from_binary functions to get an object. This ticket only affects JS Mapreduce.

russelldb commented 10 years ago

Something for the prod managers to decide (cc @gburd @michellep) I see any work to make data types work with JS MR as throwing good money after bad. New feature only MRable via Erlang MR is a good thing, imo.

russelldb commented 10 years ago

And there are helper funs for erlang MR (https://github.com/basho/riak_kv/blob/develop/src/riak_kv_crdt.erl#L30), thought maybe they should be callable from riak_kv_mapreduce

russelldb commented 10 years ago

In my opinion this is a "won't fix"

russelldb commented 10 years ago

Can I get a word from prod management (@greg @michellep @jonmeredith.)? I'm minded to just close this "won't fix"

russelldb commented 10 years ago

Engineering mumble agreed "won't fix" for 2.0-beta. Re-milestoning on 2.1 for now.