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

Avoid exclusive lock on DcpConnMap for "dcp" and "dcpagg" stats

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Duplicate
    • Major
    • None
    • 7.2.0, 7.2.1
    • couchbase-bucket
    • None
    • 0

    Description

      Follow-up to MB-56891 - while that MB did reduce the cost of generating "dcp" and "dcpagg" stats by not holding the DcpConnMap lock for the duration of the stat call (see https://review.couchbase.org/c/kv_engine/+/191090), we still need to take an exclusive lock on DcpConnMap while a copy of all elements is taken (after which the lock can be dropped).

      As suggested by Jim Walker, we can do better - for callers which do not need to modify the elements of DcpConnMap, we can add a read (shared) locking variant of DcpConnMap::forEach which can just take a shared lock on the map, take a copy, drop the (shared) lock and iterate on the copy, alowing any other actors needing read access to also iterate over it - e.g. other STAT calls.

      This appears to require more significant changes - for example we have functions called by forEach() users (e.g. DCP stats) which are not currently const; and hence need to be modified. Some of these changes are simple, but others are a bit more involved - for example either adding const support to AtomicUnorderedMap::for_each() as used by DcpConsumer::streams, or replacing that with say folly::AtomicHashMap - as we already have for DcpProducer::streams.

      Note we still need to do the lock, copy, unlock, interate idiom - the problem observed in MB-56891 was that when a Dcp connection are trying to close, it needed to update the DcpConnMap to remove the connection, but could not as another thread had taken an (exclusive) lock on the map (a Stats call) via DcpConnMap::forEach(), and for the duration of the forEach call the map could not be (exclusively) locked. As such, even a shared lock taken while iterating the map would block another thread from taking exclusive access - i.e. the Dcp close handling.

      Attachments

        Issue Links

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

          Activity

            People

              drigby Dave Rigby (Inactive)
              drigby Dave Rigby (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty