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.

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

Make Ruby driver automatically use r.json in inserts so that it's fast. #1085

Closed
mlucy opened this issue Jun 26, 2013 · 18 comments
Closed

Make Ruby driver automatically use r.json in inserts so that it's fast. #1085

mlucy opened this issue Jun 26, 2013 · 18 comments
Assignees
Milestone

Comments

@mlucy
Copy link
Member

mlucy commented Jun 26, 2013

No description provided.

@ghost ghost assigned mlucy Jun 26, 2013
@coffeemug
Copy link
Contributor

@mlucy -- could you also do a quick performance test with and without r.json and post results here?

@mlucy
Copy link
Member Author

mlucy commented Jun 27, 2013

Will do.

@mlucy
Copy link
Member Author

mlucy commented Jun 27, 2013

This is in review #675 by @AtnNn . It seems to speed up inserts of 10k-sized documents by ~30x over the old driver (by which I mean the newer old driver with the fast protobuf library).

@mlucy
Copy link
Member Author

mlucy commented Jun 27, 2013

Also, note that I only turned this on for insert, since encoding objects as JSON in normal protobuf terms messes with the backtraces and is in general more confusing than it has to be.

@coffeemug
Copy link
Contributor

It seems to speed up inserts of 10k-sized documents by ~30x

👍

@coffeemug
Copy link
Contributor

Also, note that I only turned this on for insert

To the person that did the other drivers (@jdoliner ?), do we also fall back to it on inserts only in Python and JS?

@mlucy
Copy link
Member Author

mlucy commented Jun 27, 2013

I'm not sure anyone did the other drivers yet, did they?

@coffeemug
Copy link
Contributor

I think Joe did it as part of the r.json command.

@wmrowan
Copy link
Contributor

wmrowan commented Jun 28, 2013

No, Joe added the command but did not add it to insert. Note that this is trickier than just wrapping the argument with r.json(JSON.stringify(doc)) because have to first decided if this document can be correctly serialized as json. I'm working on adding this functionality to JS and Python.

@wmrowan
Copy link
Contributor

wmrowan commented Jun 28, 2013

The JS version of this is part of branch fast_js_protobuf and which is currently in review 680.

@mlucy
Copy link
Member Author

mlucy commented Jun 29, 2013

The ruby changes are in next.

@mlucy
Copy link
Member Author

mlucy commented Jun 29, 2013

Re-assigning to @wmrowan because he seems to be tracking the JS and python changes in this issue.

@ghost ghost assigned wmrowan Jun 29, 2013
@coffeemug
Copy link
Contributor

@wmrowan -- is this functionality in next in the other drivers? Should we close this issue?

@wmrowan
Copy link
Contributor

wmrowan commented Jul 1, 2013

I haven't yet merged this because I have some misgivings about the amount of testing we've done on this functionality. I talked to @mlucy on Friday about the state of testing and while we've both confirmed performance boosts though casual perf tests neither of us has written polyglot tests covering all the edge cases.

@coffeemug
Copy link
Contributor

What's the plan for getting this in? I'd like to tag 1.7 tonight.

@wmrowan
Copy link
Contributor

wmrowan commented Jul 1, 2013

I'd like to work with @mlucy today to properly test this. I have the
feeling there are some lurking problems.

On Mon, Jul 1, 2013 at 11:52 AM, coffeemug notifications@github.com wrote:

What's the plan for getting this in? I'd like to tag 1.7 tonight.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1085#issuecomment-20302223
.

@coffeemug
Copy link
Contributor

Ok.

@wmrowan
Copy link
Contributor

wmrowan commented Jul 2, 2013

This is in next. Tests are in review 689 by @mlucy.

@wmrowan wmrowan closed this as completed Jul 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants