[SERVER-62926] Change collection lock RAII types to use TenantNamespace Created: 24/Jan/22  Updated: 06/Dec/22  Resolved: 06/Apr/22

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

Type: Task Priority: Major - P3
Reporter: Janna Golden Assignee: [DO NOT USE] Backlog - Server Serverless (Inactive)
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Serverless
Participants:

 Description   

There are a number of RAII types that take collection locks defined in both catalog_raii.h/cpp and db_raii.h,cpp, we should change these classes to use TenantNamespaceOrUUID rather than NamespaceStringOrUUID. Many of these classes wrap one another, so though there are many I think it will be easier to change them in the same commit. These classes are unit tested in both the DBRAIITestFixture and CatalogRAIITestFixture.

At this point, we should be able to write a js test that successfully runs concurrent operations on conflicting namespaces but for different tenants. The test can do something like the following:

  • Start up a single node replica set (unless SERVER-62880 is finished, then we should be able to start up a standard 3-node replica set)
  • Create two "tenants" and two connections to the primary node. Set the security token for each tenant on one of the connection (see security_token.js for an example of how to create a tenantId and set it on a conn)
  • Run concurrent requests from each connection on collections with the same name and assert each succeeds. We can use failpoints to force a hang after taking the collection lock for a request, that way we can run a request on one connection, wait to hit the failpoint, then run a request that takes a lock in a conflicting mode on the other connection and assert it succeeds, then turn off the failpoint and assert the initial request succeeds. i.e.

    // turn on failpoint that forces a hang after taking a lock during renameCollection
    // send renameCollection on tenantA's connection for the db.foo collection, wait to hit the failpoint --> rename takes the collection lock in MODE_X lock, so requesting any other lock on the same resource should fail 
    // send some other request i.e. an insert on tenantB's db.foo collection, assert this succeeds
    // turn off failpoint, assert rename was successful
    



 Comments   
Comment by Janna Golden [ 15/Feb/22 ]

dianna.hohensee yeah we did consider that, but chose to create these new types (TenantNamespace and TenantDatabaseName) and only use them in catalog data structures which must be tenant aware. However, we're now reconsidering this approach so may change course.

Comment by Dianna Hohensee (Inactive) [ 09/Feb/22 ]

I noticed the new TenantDatabaseName work done recently in SERVER-61987. Has just changing the internals of NamespaceString/OrUUID been considered, instead of changing the name/class everywhere? Rather than putting Tenant everywhere, just make existing classes Tenant aware. Similarly TenantDatabaseName could be DatabaseName – that change is already in, though, so it doesn't matter much for the time being.

I'm not particularly familiar with what you're doing with the tenant work. But I'm leery of lower level classes of the codebase being all about tenants. And it would be a ton of code churn.

Generated at Thu Feb 08 05:56:26 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.