Receive communication error when Athena returns NaN value

[kravets-levko]

Issue #2544

Solution: use simplejson instead of json to dump query results; simplejson can replace NaN/Infinity values with null

[vercel]

This pull request is automatically deployed with Now.

To access deployments, click Details below or on the icon next to each push.

[arikfr]

We should probably switch to simple_json everywhere. I just wonder how can we make sure this is the right decision?

[jezdez]

@arikfr I would argue that we should maybe look for a faster and/or better json implementation than simplejson given the amount of serialization/deserialization happening allover Redash.

There are some precedence discussing benchmarks, other benchmarks, memory consumption (!) and many, many, many more. OTOH simplejson is the real thing and has been maintained for a while now.

[arikfr]

@jezdez thank you for the pointers! You're definitely right that we will benefit from a more performant (and memory efficient) JSON implementation.

From reading the memory consumption blogpost, it looks like simplejson has the best memory consumption for a JSON that's more similar to what we usually have:

Peak memory usage (Python 2.7):

      simplejson: 1,171.7 Mb
           cjson: 1,304.2 Mb
            yajl: 1,457.1 Mb
            json: 1,468.0 Mb
       rapidjson: 1,561.6 Mb
        jsonlib2: 2,134.9 Mb
         jsonlib: 2,137.0 Mb
           ujson: 2,149.9 Mb

Also based on the same blog post, it seems like that only relevant alternative is ujson (considering stability), but it doesn't seem to support all the options we need...

[jezdez]

@arikfr Excellent, I hadn't looked as closely, that seals it. Do you want me to work on a patch?

[arikfr]

You're always welcome to :)