Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: 10.0.4
    • Labels:
      None
    • Global Rank:
      3231

      Description

      Requiremen: when this task is completed, get_table_share() shall not acquire LOCK_open.

      Note: this task is not aiming to get rid of LOCK_open completely, further optimizations will be subject of other task(s).

      New mutex shall be added to protect table specific variables. Suggested name: TABLE_SHARE::LOCK_table_share. We agreed not to use TABLE_SHARE::LOCK_ha_data because it is supposed to be used by storage engines and they may lock it for a long time.

      get_table_share() uses the following variables protected by LOCK_open:
      table_def_cache
      table_def_shutdown_in_progress
      last_table_id
      TABLE_SHARE::ref_count
      TABLE_SHARE::next
      TABLE_SHARE::prev
      TABLE_SHARE::m_flush_tickets
      end_of_unused_share
      oldest_unused_share
      refresh_version

      1. table_def_cache: Monty suggests to test rw-locks first and migrate to lf-hash later, Serg suggests lf-hash.

      Svoj: for initial implementation we will use rw-locks, there are three open questions with lf_hash that may delay implementation: 1) safe iteration (snapshot), 2) unsafe iteration, 3) how to safely retrieve elements count. It should be easy to switch to lf_hash when they're solved.

      2. table_def_shutdown_in_progress: not used directly by get_table_share(), but needs to be taken into account to be able to fix other functions easier. Updated only during shutdown when all client connections are closed. No lock should be needed, even need of this variable is questionable: why not to update refresh_version instead?

      Svoj: this variable doesn't need locking indeed as it is used only during shutdown. We can't replace it's function by updating refresh_version, but we can replace it by setting table_def_size to 0.

      3. last_table_id: simple id generator, use atomics.

      4. TABLE_SHARE::ref_count: shall be protected by TABLE_SHARE::LOCK_table_share.

      5. TABLE_SHARE::next, ::prev, oldest_unused_share, end_of_unused_share: these variables are intended to effeciently free oldest share first. Shall be protected by global mutex: LOCK_unused_shares.

      6. TABLE_SHARE::m_flush_tickets shall be protected by TABLE_SHARE::LOCK_table_share. It is needed to avoid LOCK_open when removing share from cache (e.g. purging unused shares in get_table_share() and release_table_share()).

      7. refresh_version: use atomics.

      Per request from Igor, Svoj analyzed if table_cache.cc provides black-box clean API to table definition cache. Unfortunately it does not give significant benefit over existing API and may complicate implementation of this task.

        Issue Links

          Activity

          Hide
          Michael Widenius added a comment -
          • I am ok with using lf_hash, assuming that it's not more work than 4-5 hours to use lf_hash instead of a rw_lock.
          • Yes, we can replace table_def_shutdown_in_progress with incrementing refresh_version.
          • Yes, we can change last_table_id to use atomics
          • Ok to spend up a 6 hours in providing a class to handle table_def_cache + unused lists + open share variables. Not critical as this code is not regarded to be reusable or likely to be significantly better than normal code.
          Show
          Michael Widenius added a comment - I am ok with using lf_hash, assuming that it's not more work than 4-5 hours to use lf_hash instead of a rw_lock. Yes, we can replace table_def_shutdown_in_progress with incrementing refresh_version. Yes, we can change last_table_id to use atomics Ok to spend up a 6 hours in providing a class to handle table_def_cache + unused lists + open share variables. Not critical as this code is not regarded to be reusable or likely to be significantly better than normal code.
          Hide
          Sergey Vojtovich added a comment -

          Pushed to 10.0.4.
          revno: 3793
          revision-id: svoj@mariadb.org-20130814084850-b6gi2ipymsn4n2me

          Show
          Sergey Vojtovich added a comment - Pushed to 10.0.4. revno: 3793 revision-id: svoj@mariadb.org-20130814084850-b6gi2ipymsn4n2me

            People

            • Assignee:
              Sergey Vojtovich
              Reporter:
              Sergey Vojtovich
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1 week, 4 days Original Estimate - 1 week, 4 days
                1w 4d
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 2 weeks, 3 days, 4 hours
                2w 3d 4h