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

Cannot make persistent change to num nonio/auxio threads

    XMLWordPrintable

Details

    • Untriaged
    • 1
    • Yes
    • KV 2021-Dec, KV 2022-Jan

    Description

      Config params num_nonio_threads / num_auxio_threads set via ns_server extra_config_string are not respected.

      cbepctl set flush_param still works, modifying the running config in a non-persistent fashion.

      extra_config_string in general makes persistent changes to the config for a given bucket, which will survive a restart. This is a somewhat "canonical" method provided to customers by support to change settings in response to issues. Customers with these parameters currently set would find they no longer apply if upgrading to a build in the current state.

      Code context:

      kv_engine/daemon/memcached.cc

      709
      static void startExecutorPool() {
      710
          LOG_INFO_RAW("Start executor pool");
      711
          ExecutorPool::create(ExecutorPool::Backend::Folly,
      712
                               0,
      713
                               ThreadPoolConfig::ThreadCount(
      714
                                       Settings::instance().getNumReaderThreads()),
      715
                               ThreadPoolConfig::ThreadCount(
      716
                                       Settings::instance().getNumWriterThreads()));
      

      kv_engine/engines/ep/src/ep_engine.cc

      2098
          // Ensure (global) ExecutorPool has been created, and update the (local)
      2099
          // configuration with the actual number of each thread type we have (config
      2100
          // params are typically defaulted to "0" which means "auto-configure
      2101
          // thread counts"
      2102
          auto threads = serverApi->core->getThreadPoolSizes();
      2103
          configuration.setNumReaderThreads(static_cast<int>(threads.num_readers));
      2104
          configuration.setNumWriterThreads(static_cast<int>(threads.num_writers));
      2105
       
      2106
          auto* pool = ExecutorPool::get();
      2107
          // Update configuration to reflect the actual number of threads which have
      2108
          // been created.
      2109
          configuration.setNumReaderThreads(pool->getNumReaders());
      2110
          configuration.setNumWriterThreads(pool->getNumWriters());
      2111
          configuration.setNumAuxioThreads(pool->getNumAuxIO());
      2112
          configuration.setNumNonioThreads(pool->getNumNonIO());
      

      This abandoned patch would move to these values being controlled by memcached settings, rather than bucket config, which makes sense as these have global impact.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            The ns_server fix broke upgrades. The new settings num_auxio_threads and num_nonio_threads shouldn't be added until the cluster_compat_mode is NEO. Here's an example in mad-hatter http://src.couchbase.org/source/xref/mad-hatter/ns_server/src/menelaus_web_mcd_settings.erl#32-54

            steve.watanabe Steve Watanabe added a comment - The ns_server fix broke upgrades. The new settings num_auxio_threads and num_nonio_threads shouldn't be added until the cluster_compat_mode is NEO. Here's an example in mad-hatter http://src.couchbase.org/source/xref/mad-hatter/ns_server/src/menelaus_web_mcd_settings.erl#32-54

            I chatted with Dave F. about this and he asked me to make the change.

            steve.watanabe Steve Watanabe added a comment - I chatted with Dave F. about this and he asked me to make the change.
            dfinlay Dave Finlay added a comment -

            Steve Watanabe: lovely work finding and debugging this issue. (For folks following along, it turns out that this patch actually breaks offline ugprade.) Steve - I will assign to you; I think it's too much to ask folks from outside of ns_server to navigate the complexities of cluster compat mode upgrade.

            dfinlay Dave Finlay added a comment - Steve Watanabe : lovely work finding and debugging this issue. (For folks following along, it turns out that this patch actually breaks offline ugprade.) Steve - I will assign to you; I think it's too much to ask folks from outside of ns_server to navigate the complexities of cluster compat mode upgrade.

            Dave opened a separate bug MB-50405 to track the upgrade issue.

            steve.watanabe Steve Watanabe added a comment - Dave opened a separate bug MB-50405 to track the upgrade issue.

            Build couchbase-server-7.1.0-2062 contains ns_server commit a0557f1 with commit message:
            MB-50405, MB-49977 Upgrade memcached_defaults

            build-team Couchbase Build Team added a comment - Build couchbase-server-7.1.0-2062 contains ns_server commit a0557f1 with commit message: MB-50405 , MB-49977 Upgrade memcached_defaults

            People

              steve.watanabe Steve Watanabe
              james.harrison James Harrison
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  PagerDuty