Uploaded image for project: 'Couchbase Server'
  1. Couchbase Server
  2. MB-51345

ep_testsuite "mem stats" fails if CB_DEVELOPMENT_ASSERTS=OFF

    XMLWordPrintable

Details

    • 1

    Description

      Affects KV Neo CV.

      This development assertion:

      bool CheckpointMemRecoveryTask::runInner() {
      ...
          case CheckpointRemoval::Eager: {
      #if CB_DEVELOPMENT_ASSERTS
              // if eager checkpoint removal has been configured, calling
              // attemptCheckpointRemoval here should never, ever, find any
              // checkpoints to remove; they should always be removed as soon
              // as they are made eligible, before the lock is released.
              // This is not cheap to verify, as it requires scanning every
              // vbucket, so is only checked if dev asserts are on.
              Expects(attemptCheckpointRemoval().second == 0);
      #endif
              break;
          }
      

      Was essentially expected to be a no-op, as Eager checkpoint removal ensures that no unreffed checkpoints remain in the checkpoint manager.

      However,

      CheckpointMemRecoveryTask::attemptCheckpointRemoval() {
      ...
      memReleased +=
                      vb->checkpointManager->removeClosedUnrefCheckpoints().memory;
      

      CheckpointManager::ReleaseResult
      CheckpointManager::removeClosedUnrefCheckpoints() {
      ...
          CheckpointList toRelease;
          {
              std::lock_guard<std::mutex> lh(queueLock);
              maybeCreateNewCheckpoint(lh);
              toRelease = extractClosedUnrefCheckpoints(lh);
          }
      

      It also has the potential side effect of closing the current open checkpoint. Once CB_DEVELOPMENT_ASSERTS was set to OFF for neo, this side effect would no longer occur.

      ep_testsuite mem stats test fails if this side effect does not occur:

      static enum test_result test_mem_stats(EngineIface* h) {
      ...
          int itemsRemoved = get_int_stat(h, "ep_items_rm_from_checkpoints");
          wait_for_persisted_value(h, "key", value.c_str());
          testHarness->time_travel(65);
          if (isPersistentBucket(h)) {
              wait_for_stat_change(h, "ep_items_rm_from_checkpoints", itemsRemoved);
          }
      

      This will timeout, as nothing triggers a checkpoint close, so the checkpoint will not yet be removed routinely.

      KV behaviour should not be visibly changed by changing CB_DEVELOPMENT_ASSERTS (aside from if the guarded assertion fails, of course).

      Additionally, the test should not rely on this side effect.

      Attachments

        For Gerrit Dashboard: MB-51345
        # Subject Branch Project Status CR V

        Activity

          People

            james.harrison James Harrison (Inactive)
            james.harrison James Harrison (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty