PERF-2676 TPCC fails for MongoDB on Atlas #10
Conversation
pytpcc/drivers/mongodbdriver.py
Outdated
| self.result_doc.update(result_doc) | ||
| self.result_doc['after']=self.get_server_status() | ||
| self.client.test.results.save(self.result_doc) | ||
| self.client.test.results.save(self.result_doc, check_keys=False) |
There was a problem hiding this comment.
It looks like save is deprecated. should this be an insert_one or replace_one?
|
Here is the variant with insert_one. Here is the patch. But we need new Pymongo 3.12.1 - otherwise we get the same error in Atlas with MongoDb 5.0 (we have currently 3.11.1 in workload_setup.common.yml). It appears that we officially need 3.12.1 to support MongoDB 5.0 - so it is probably the right thing to do. The suggestion is to use insert_one as advised by the Python driver team and update Pymongo to 3.12.1 (for which I will create another PR in dsi if we agree on this approach). Any thoughts? |
| self.result_doc.update(result_doc) | ||
| self.result_doc['after']=self.get_server_status() | ||
| self.client.test.results.save(self.result_doc) | ||
| self.client.test.results.insert_one(self.result_doc) |
There was a problem hiding this comment.
I don't think this is correct, save is either an insert or a replace. The logic above only mimics the first part.
Should this be an upsert (see here)
There was a problem hiding this comment.
@jimoleary That is correct, it does formally change the logic. The literal replacement would be with the whole code you linked above. However as far as I was looking into what is going on here, it appears that only insert happens here - there is nothing to update. Unless I missed something here, why we should process update_one logic here if it never happens?
There was a problem hiding this comment.
I'm not as familiar with this code so I'll defer to you. But can we add a comment to say that this is the case?
There was a problem hiding this comment.
added a comment - let me know if it helps
There was a problem hiding this comment.
Comment looks good. I don't believe we ever set an explicit _id field in result_doc, so this should always be an insert.
There was a problem hiding this comment.
I'm also not a fan of saving the results back into the database itself, but that's a separate issue.
|
Actually we have pymongo upgrade in another PR |
| self.result_doc.update(result_doc) | ||
| self.result_doc['after']=self.get_server_status() | ||
| self.client.test.results.save(self.result_doc) | ||
| self.client.test.results.insert_one(self.result_doc) |
There was a problem hiding this comment.
Comment looks good. I don't believe we ever set an explicit _id field in result_doc, so this should always be an insert.
| self.result_doc.update(result_doc) | ||
| self.result_doc['after']=self.get_server_status() | ||
| self.client.test.results.save(self.result_doc) | ||
| self.client.test.results.insert_one(self.result_doc) |
There was a problem hiding this comment.
I'm also not a fan of saving the results back into the database itself, but that's a separate issue.
A short-term fix for py-tpcc to allow it to run on Atlas. See details on PERF-2676.