[SERVER-30485] Remove ns field from DocID for rollback with UUIDs Created: 02/Aug/17  Updated: 06/Dec/22  Resolved: 14/Dec/17

Status: Closed
Project: Core Server
Component/s: Replication
Affects Version/s: Backlog
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Allison Chang Assignee: Backlog - Replication Team
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Replication
Participants:

 Description   

Currently in rs_rollback.h, we are maintaining the namespace of documents that are being rolled back through an insert/delete/update, in the DocID struct. This field is still necessary although we are refetching documents using the UUID because it is used in

 OldClientContext ctx(opCtx, doc.ns) 

. It is likely that we can remove this doc.ns field, but further investigation into why OldClientContext is necessary for rollback is needed before removing the field. We would like the doc.ns field in the future.

Currently, we use the doc.ns field to retrieve the database level lock, when the collection itself might not exist because it has been removed from the sync source. We need to use the doc.ns field since if we attempt to use lookupNSSByUUID, we will end up with an empty namespace string, which will cause an error to be thrown when we attempt to retrieve the database lock. This should functionally be safe because we are locking the database that the collection used to be in, and UUIDs are maintained within the database.



 Comments   
Comment by Judah Schvimer [ 07/Dec/17 ]

Upon further investigation, it appears that when we tried to remove the 'ns' field, two unittests failed because collections they expected to exist had not been created. Fixing the unittests fixed the problem and the patch generally went green. As mentioned above, I don't think this is a bug, and I now don't think it's masking any bugs. See diff here for a POC: https://evergreen.mongodb.com/version/5a283fb5e3c33129d001c3e0

Comment by Judah Schvimer [ 05/Dec/17 ]

Currently, we use the doc.ns field to retrieve the database level lock, when the collection itself might not exist because it has been removed from the sync source. We need to use the doc.ns field since if we attempt to use lookupNSSByUUID, we will end up with an empty namespace string, which will cause an error to be thrown when we attempt to retrieve the database lock.

I'm confused by this because we're looking up the namespace on the local node, not on the sync source. We should never be looking up a UUID that doesn't exist locally because we remove documents from docsToRefetch that have UUIDs that are being dropped as part of rollback. It's possible this was caused by a different bug that we accidentally ignored, or I'm just forgetting the real reason. If the namespace is in fact sometimes empty, I also do not remember why we could not check if the namespace was empty before taking the database lock.

This does still appear to be safe, since a UUID cannot switch databases. Thus even if it's been renamed, the lock is still being taken on the correct database and then we are getting the collection object by UUID and only using the namespace for logging.

Generated at Thu Feb 08 04:23:59 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.