MongoDBClient cleanup #351
Conversation
… OptionsSupport class for MongoDB.
Simplified insert CRUD code to always use insertMany even when batchSize == 1, as there is no performance penalty for this in the driver.
|
I see that the Travis tests fail. This is because I removed the special case for insert with batchSize of 1 to do an insertOne, instead of a replaceOne with upsert. I believe this is proper (since batchSize > 1 uses insert rather than upsert, and the tests pass so long as you drop the collection prior to each run. Is there any way to configure these tests so that the collection is dropped between each run? |
|
Jeff - Sorry for the delay. I'll start looking this over now. |
| /** The database name to access. */ | ||
| /** The options to use for inserting many documents */ | ||
| protected static final InsertManyOptions INSERT_MANY_OPTIONS = | ||
| new InsertManyOptions().ordered(false); |
There was a problem hiding this comment.
Can this be renamed to show what it stands for instead of the type of object it is. Maybe INSERT_UNORDERED?
|
Jeff - On the Insert vs. Upsert: That was a community contribution that I think was done for usability (e.g., reloads on the same workload without a drop of the collection just work). @busbey - Do you know if we have a standard for if the insert operation should be a pure insert or if it can be an upsert (if the DB supports it)? Rob. |
|
On the Insert vs Upsert: This usability change only applies when batch size == 1, since it's using a bulk insert when batch size > 1, not a bulk upsert. |
|
We tend to give the binding implementors a lot of leeway. We've been discussing setting expectations over on #245 and #334. No one has mentioned choice of operations yet. Personally, I'd prefer if we included some means to say "truncate the table" for each binding. That would make it easier to implement a quickstart for our recommended workload ordering. |
|
I can go back to an upsert if that's the only way forward to get the tests passing again, but I'd prefer a better solution. |
|
Jeff - Will the insert be faster than the upsert? If so then lets leave the insert in place. I think the async and non-async test cases are colliding with each other since they use static ids. I think the easiest think to do is to add a truncate() method to the two clients and then call that to the setUp() for MongoDbClientTest and AsyncMongoDbClientTest. Thoughts? Rob |
|
Could they use different ids? The truncate will cause them to interfere with each other if we turn on parallel tests. |
…connection string will contain both "j" and "journal", until such time as the MongoDB Java driver supports "journal".
…e update with upsert instead of insert when batch size is 1.
|
Updated. I added journal=true back into the URL in addition to j=true, so that both drivers continue to work. I also reverted to using upsert instead of insert when the batch size is 1 so that the tests are passing once again. |
|
@busbey - Any problem with me merging this? |
|
Fine by me! |
|
Rebased and Merged. |
I reviewed the changes to the MongoDBClient class that update it to use the 3.0 Java driver, and found a number of issues, all of which are addressed in the commits in this pull request. Most are changes that simplify the MongoDBClient code in order to improve readability and reduce repetition. The only functional change should be the fix to a bug in the way that journaling is enabled in the MongoClientURI.