Info

[SERVER-32148] make NamespaceNotFound an acceptable error for emptyCapped and convertToCapped

Top User Comments

siyuan.zhou@10gen.com commented on Wed, 1 Aug 2018 20:25:52 +0000: tess.avitabile, yes, I think this issue also exists on 3.6 and 3.4. We didn't change a lot in this area so it should be a clean backport mod some test changes. xgen-internal-githook commented on Tue, 31 Jul 2018 22:30:47 +0000: Author: {'username': 'cramaechi', 'name': 'Chibuikem Amaechi', 'email': 'cramaechi@me.com'} Message: SERVER-32148 Make NamespaceNotFound an acceptable error for emptyCapped and convertToCapped Closes #1206 Signed-off-by: Siyuan Zhou Branch: master https://github.com/mongodb/mongo/commit/35757e8ef3134fad1a09cb09a69882929d9ebb76 siyuan.zhou@10gen.com commented on Fri, 26 Jan 2018 05:15:20 +0000: cramaechi, thanks for the pull request! I commented on it. The format looks fine to me. I believe we need to ASSERT_OK() those commands can be applied correctly even if the collection doesn't exist. You were right that we need to change one more place. emptyCapped should uassert with ErrorCodes::NamespaceNotFound instead of the location number. Also uassert seems more consistent here than massert. Could you please update the pull request? I'd be happy to review the change. cramaechi commented on Wed, 24 Jan 2018 14:31:08 +0000: Hi Siyuan, A pull request for this ticket has been opened. I ran into a problem while running sync_tail_test.cpp with my two new test cases. Specifically, test case ConvertToCappedNamespaceNotFound failed due to ASSERT_NOT_OK() being used instead of ASSERT_OK(). I'm almost certain that the root cause of the error is the BSON command I constructed. cramaechi commented on Tue, 23 Jan 2018 12:09:42 +0000: Hi Siyuan - I will try to open a pull request by the end of today. Thank you! - Chibuikem. A siyuan.zhou@10gen.com commented on Tue, 23 Jan 2018 03:34:47 +0000: Hi cramaechi, it would be great if you could make a pull request for this. Thanks! After you make the change we discussed above, I'd suggest adding new tests cases for both commands in sync_tail_test.cpp. An example test case is this one. You may not want to createCollection() at the beginning, but the structure will probably look very similar. Beyond the returned result, you might also want to check the collection isn't created at all. An example of accessing database and collection is here. The code needs to check autoColl.getDb() and autoColl.getCollection() are both nullptr. Adding comments explaining the test cases is always appreciated. Please let me know if you have any questions. Thanks again! spencer commented on Thu, 18 Jan 2018 23:01:03 +0000: I'm happy to take a pull request on this! Would be great to get this done without pulling away from our other development work too badly. siyuan.zhou, can you point Chibuikem in the direction of the unit tests he would need to write/extend to test this change? cramaechi commented on Thu, 18 Jan 2018 08:11:13 +0000: Hi Siyuan, Sounds good. I'll await your team's decision on how best to move forward with this ticket. - Chibuikem. A siyuan.zhou@10gen.com commented on Wed, 17 Jan 2018 22:33:06 +0000: Hi Chibuikem, Great! I discussed this with our team, we believe the right solution is to add "NamespaceNotFound" as an acceptable error to ignore for secondary oplog application in oplog.cpp here. ramon.fernandez and spencer, how do you want this ticket proceed? We can fix this directly or work on the pull request with Chibuikem. siyuan.zhou@10gen.com commented on Wed, 17 Jan 2018 02:48:21 +0000: cramaechi, thanks for your comment and your interests in contribution! Just to make things clear and to address your issues first. Did this issue happen to you? On primary or secondary? How did it happen? What version are you using?

Steps to Reproduce