Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-84275

Change tenant ID feature flag to use isEnabled

    • Type: Icon: Task Task
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • Service Arch
    • 3

      Follow up to SERVER-82246. We changed some tenant ID feature flags to use isEnabledUseLastLTSFCVWhenUninitialized since they could be run during startup/when FCV is uninitialized. However, ideally we should figure out a way to just use isEnabled because ideally we don't want to turn off the tenant ID feature when it actually should be turned on.

      Specifically, in database_name_util.cpp and namespace_string_util.cpp's serialize/deserializeForStorage functions, they might invariant if called during startup. We should avoid checking the feature flag if this is not a tenant specific collection/if there is no tenant id by doing something like this:

      std::string DatabaseNameUtil::serializeForStorage(const DatabaseName& dbName,
                                                        const SerializationContext& context) {
          auto fcv = serverGlobalParams.featureCompatibility.acquireFCVSnapshot();
          if (!fcv.isVersionInitialized()) {
              invariant(!dbName.tenantId());
              return dbName.toString();
          }
          if (gFeatureFlagRequireTenantID.isEnabled(fcv)) {
              return dbName.toString();
          }
          return dbName.toStringWithTenantId();
      }
      
      @@ -195,10 +199,18 @@ DatabaseName DatabaseNameUtil::deserializeForAuthPrevalidated(boost::optional<Te
       DatabaseName DatabaseNameUtil::deserializeForStorage(boost::optional<TenantId> tenantId,
                                                            StringData db,
                                                            const SerializationContext& context) {
      -    if (gFeatureFlagRequireTenantID.isEnabled(
      -            serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) {
      -        if (!DatabaseName::kAdmin.dbNameEqual(db) && !DatabaseName::kLocal.dbNameEqual(db) &&
      -            !DatabaseName::kConfig.dbNameEqual(db) && !DatabaseName::kExternal.dbNameEqual(db))
      +    auto isTenantAgnosticDb = [](StringData db) {
      +        return DatabaseName::kAdmin.dbNameEqual(db) || DatabaseName::kLocal.dbNameEqual(db) ||
      +            DatabaseName::kConfig.dbNameEqual(db) || DatabaseName::kExternal.dbNameEqual(db);
      +    };
      +
      +    auto fcv = serverGlobalParams.featureCompatibility.acquireFCVSnapshot();
      +    if (!fcv.isVersionInitialized()) {
      +        invariant(isTenantAgnosticDb(db) && !tenantId);
      +        return DatabaseName(boost::none, db);
      +    }
      +    if (gFeatureFlagRequireTenantID.isEnabled(fcv)) {
      +        if (!isTenantAgnosticDb(db))
                   uassert(7005300, "TenantId must be set", tenantId != boost::none);
      

      However, it also seems like there are some tenant specific namespaces that are being run through these functions during startup. In this patch with the above changes, the native_tenant_data_isolation_stop_restart.js test failed with an invariant, as it was a tenant specific collection, with a tenant ID. I believe this happened when we stopped and restarted the replica set, and load the catalog again with already existing views

            Assignee:
            backlog-server-servicearch [DO NOT USE] Backlog - Service Architecture
            Reporter:
            huayu.ouyang@mongodb.com Huayu Ouyang
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: