Reset disagg block cache parameter globals in PaliCallbacksTest::tearDown to prevent test state leakage

XMLWordPrintableJSON

    • Type: Task
    • Resolution: Fixed
    • Priority: Minor - P4
    • 9.0.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • None
    • Storage Engines - Persistence
    • Fully Compatible
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      Issue Summary

      While adding unit tests for the disaggregated block cache sizing parameters ([SERVER-128863]), we found that several existing tests in pali_test.cpp mutate process-global server parameters and never restore them, leaking state across tests within the unit-test binary.

      Context

      • Affected file: src/mongo/db/modules/atlas/src/disagg_storage/pali/block_cache/pali_test.cpp
      • The PaliCallbacksTest tests GetFullPageFromCache, GetDeltaPageFromCache, and GetMultiplePagesFromCache set gBlockCacheSizeBytes (1MB) and gBlockCacheNumShards (1) but never reset them. The fixture's setUp/tearDown does not reset them either.
      • These are process-global server parameters (declared in disagg_storage/server_parameters.idl), so the values persist for the remainder of the test binary after any of these tests run.
      • No current test depends on the default (0) value, so this has not yet caused a failure. However, the MongoDB unittest runner can randomize test order, so a future test relying on the defaults could fail intermittently depending on ordering. This is a latent global-state leak.
      • Found during review of the block cache percentage-sizing work ([SERVER-128863], epic WT-17800).

      Proposed Solution

      Reset the parameters once in PaliCallbacksTest::tearDown() rather than guarding each test individually:

      void tearDown() override {
          // ... existing teardown ...
          // Block cache parameters are process globals; reset so they don't leak between tests.
          gBlockCacheSizeBytes = 0;
          gBlockCacheSizePct = 0;
          gBlockCacheNumShards = 0;
          ServiceContextMongoDTest::tearDown();
      }
      
      • This covers all current and future fixture-based tests uniformly and fixes the three leaking tests for free.
      • Remove the now-redundant per-test parameter resets from fixture-based tests. The one non-fixture test (BlockCacheParamsTest.SizeBytesAndSizePctAreMutuallyExclusive) is not on the fixture and keeps its own ScopeGuard.

      Definition of Done

      • PaliCallbacksTest::tearDown() resets gBlockCacheSizeBytes, gBlockCacheSizePct, and gBlockCacheNumShards.
      • Redundant per-test parameter resets removed from fixture-based tests.
      • pali_test passes, including under randomized test ordering.

            Assignee:
            Etienne Petrel
            Reporter:
            Etienne Petrel
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: