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

Async mongodb driver #180

Merged
merged 28 commits into from May 24, 2015

Conversation

Projects
None yet
6 participants
@allanbank
Copy link
Collaborator

allanbank commented Jun 20, 2014

This pull request contains a new driver for MongoDB based on the asynchronous driver. The bulk of the changes are adding the required logic to the startup scripts to support the second driver. I have used this branch to run a series of benchmarks.

There are also a number of community contributed updates to the old MongoDB Driver. These include:

  • Updated README with specific instructions for running a MongoDB suite.
  • Updates to the configuration to now be strictly based on a MongoDB Connection String URI which is standardized across drivers.
  • Other small fixes and performance enhancements.

This pull request will supersede the following pull requests:

  • PR-112 - Integrates the MongoDB Connection String URI for both drivers.
  • PR-115 - The writeConcern and readPreference are expressed via the standardized MongoDB Connection String URI.
  • PR-131 - Read preferences are passed via the MongoDB Connection String and replica sets are automatically detected for the asynchronous driver and will be enabled with the MongoDB Inc. driver by passing multiple servers in the URI.

I know this is a large volume of changes but they are all focused on the MongoDB driver and thought it would be easier to provide them in one batch than a bunch of pull requests. If you would prefer a bunch of smaller changes I will need guidance on the desired granularity.

@allanbank

This comment has been minimized.

Copy link
Collaborator

allanbank commented Jun 21, 2014

Note says that the Travis Build Failed but it appears to be an issue with Travis not being able to download artifacts from Maven. Can someone trigger a manual rebuild?

@westonplatter

This comment has been minimized.

Copy link
Contributor

westonplatter commented Jun 21, 2014

@allanbank I bumped if for a rebuild (even though the Github status does not show the reubuild).

@busbey

This comment has been minimized.

Copy link
Collaborator

busbey commented May 20, 2015

I restarted the build now that the mapkeeper stuff is out of the way. I'd like to get this set of changes merged in before cutting the next release.

If someone from the mongo community can work with me on needed updates, I'll proactively go point the related open issues here and close them.

@@ -107,9 +108,15 @@ command = COMMANDS[sys.argv[1]]["command"]
database = sys.argv[2]
db_classname = DATABASES[database]
options = sys.argv[3:]
java_home = os.environ["JAVA_HOME"]

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

the java home stuff is good, but seems unrelated to the rest of the mongo improvements. If I file an issue for "allow the use of java home" could you edit the commit message to call out closing it?

This comment has been minimized.

@allanbank

allanbank May 20, 2015

Collaborator

I wrote an issue: #252.

This comment has been minimized.

@busbey

busbey May 21, 2015

Collaborator

great. presuming you go forward with the rebase, please edit the commit message for this part of the changes to note "fixes #252"


Clone the YCSB git repository and compile:
### 2. Install Java and Maven

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

This sounds like a gap in our overall YCSB docs. individual db bindings shouldn't have to tell folks to go get java.

I'll write a follow on issue to deal with this across modules.

@@ -0,0 +1,477 @@
/**
* Copyright (c) 2013-2014, Allanbank Consulting, Inc. All rights reserved.

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

This will take some time for me to review how our licensing ought to work. Generally, for the contributions I've already seen to the project, the practice is for contributors to assign copyrights to Yahoo! (as the project originator) and then use an ASL license.

This comment has been minimized.

@allanbank

allanbank May 20, 2015

Collaborator

I will update the license. Is there a assignment document that need to be submitted?

This comment has been minimized.

@busbey

busbey May 21, 2015

Collaborator

let me touch base with other maintainers. I can't find anything in the project docs.

* </p>
*
* @author rjm
* @copyright 2013-2014, Allanbank Consulting, Inc., All Rights Reserved

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

the project relies on license headers at the top of files and not copyright javadocs.

* BinaryByteArrayIterator provides an adapter from a {@link BinaryElement}
* to a {@link ByteIterator}.
*
* @copyright 2013, Allanbank Consulting, Inc., All Rights Reserved

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

definitely no need for another copyright notice down here.

String url = props.getProperty("mongodb.url",
"mongodb://localhost:27017");
database = props.getProperty("mongodb.database", "ycsb");
String writeConcernType = props.getProperty("mongodb.writeConcern",

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

since we're moving the write concern from a property to a connection URL, these changes will have to go into a new major version of YCSB since people with existing test rigs for running MongoDB will break.

That's fine, just a heads up about timing.

This comment has been minimized.

@allanbank

allanbank May 20, 2015

Collaborator

I will make it backwards compatible and note that the URL supersedes the old properties.

This comment has been minimized.

@busbey

busbey May 21, 2015

Collaborator

perfect!

pom.xml Outdated
@@ -50,7 +50,8 @@
<infinispan.version>7.1.0.CR1</infinispan.version>
<openjpa.jdbc.version>2.1.1</openjpa.jdbc.version>
<mapkeeper.version>1.0</mapkeeper.version>
<mongodb.version>2.11.2</mongodb.version>
<mongodb.version>2.12.2</mongodb.version>

This comment has been minimized.

@busbey

busbey May 20, 2015

Collaborator

can we update this to a more recent version? I know #221 was trying to go to 2.13.0

This comment has been minimized.

@allanbank

allanbank May 20, 2015

Collaborator

I will update to the latest version.

This comment has been minimized.

@busbey

busbey May 21, 2015

Collaborator

just a heads up, I'd like to merge #253 while this stuff is in flight since it's so small.

This comment has been minimized.

@allanbank

allanbank May 21, 2015

Collaborator

OK - let me know when that is merged done.

This comment has been minimized.

@busbey

busbey May 21, 2015

Collaborator

done. :)

@allanbank

This comment has been minimized.

Copy link
Collaborator

allanbank commented May 20, 2015

@busbey - Give me a day or two to make the changes you suggested. Is it OK if I rebase the pull on the latest master making the changes as I go?

@busbey

This comment has been minimized.

Copy link
Collaborator

busbey commented May 21, 2015

please feel free to rebase. I prefer rebases to lots of merges, personally. :)

@allanbank allanbank force-pushed the allanbank:async-mongodb-driver branch from a243b47 to 6970ee1 May 22, 2015

@allanbank

This comment has been minimized.

Copy link
Collaborator

allanbank commented May 22, 2015

@busbey -

I think that latest push covers all of the issues you raised. Let me know if I missed something.

Rob.

@busbey

This comment has been minimized.

Copy link
Collaborator

busbey commented May 23, 2015

The update looks great.

It looks like with the current update we'll be using the Mongo 3 client, right? Will that prevent us from talking to Mongo 2 servers? If so, how comon are Mongo 2 installations? Is it worth maintaining both, ala our HBase binding?

@allanbank

This comment has been minimized.

Copy link
Collaborator

allanbank commented May 23, 2015

The driver can talk to version 2 and 3 clusters.

Rob

On May 23, 2015 12:22:12 PM EDT, Sean Busbey notifications@github.com wrote:

The update looks great.

It looks like with the current update we'll be using the Mongo 3
client, right? Will that prevent us from talking to Mongo 2 servers? If
so, how comon are Mongo 2 installations? Is it worth maintaining both,
ala our HBase binding?


Reply to this email directly or view it on GitHub:
#180 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

busbey added a commit that referenced this pull request May 24, 2015

Merge pull request #180 from allanbank/async-mongodb-driver
[Mongodb] merge adds Async mongodb driver, update normal driver to talk to mongo 2 and 3 clusters.

@busbey busbey merged commit 0ba7e79 into brianfrankcooper:master May 24, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@busbey

This comment has been minimized.

Copy link
Collaborator

busbey commented May 24, 2015

Thanks for sticking with this contribution! I hope to see more great stuff from the mongo community in the future.

@allanbank

This comment has been minimized.

Copy link
Collaborator

allanbank commented May 27, 2015

@busbey - No problem. Thanks for doing the merge.

I mentioned it in the initial pull request but I am pretty certain these pull requests can be closed now.

  • PR-112 - Integrates the MongoDB Connection String URI for both drivers.
  • PR-115 - The writeConcern and readPreference are expressed via the standardized MongoDB Connection String URI.
  • PR-131 - Read preferences are passed via the MongoDB Connection String and replica sets are automatically detected for the asynchronous driver and will be enabled with the MongoDB Inc. driver by passing multiple servers in the URI.

If you would like me to add an explicit comment to each of those explaining why it can be closed I'll be happy to do that.

Rob.

@busbey

This comment has been minimized.

Copy link
Collaborator

busbey commented May 27, 2015

Thanks for the reminder, I'll go close them out.

@allanbank allanbank deleted the allanbank:async-mongodb-driver branch May 28, 2015

jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016

Merge pull request brianfrankcooper#180 from allanbank/async-mongodb-…
…driver

[Mongodb] merge adds Async mongodb driver, update normal driver to talk to mongo 2 and 3 clusters.

jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016

Merge pull request brianfrankcooper#180 from allanbank/async-mongodb-…
…driver

[Mongodb] merge adds Async mongodb driver, update normal driver to talk to mongo 2 and 3 clusters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment