Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-4411

Kudu inserts violate lock ordering and could deadlock

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: Impala 2.8.0
    • Fix Version/s: Impala 2.8.0
    • Component/s: Distributed Exec
    • Labels:

      Description

      "IMPALA-3788: Support for Kudu 'read-your-writes' consistency" introduced a violation of the lock ordering in ImpalaServer:

        unique_lock<mutex> l(lock_);
        end_time_ = TimestampValue::LocalTime();
        summary_profile_.AddInfoString("End Time", end_time().DebugString());
        summary_profile_.AddInfoString("Query State", PrintQueryState(query_state_));
        query_events_->MarkEvent("Unregister query");
      
        if (coord_.get() != NULL) {
          // Update latest observed Kudu timestamp stored in the session.
          uint64_t latest_kudu_ts = coord_->GetLatestKuduInsertTimestamp();
          if (latest_kudu_ts > 0) {
            VLOG_RPC << "Updating session latest observed Kudu timestamp: " << latest_kudu_ts;
            lock_guard<mutex> session_lock(session_->lock);
            session_->kudu_latest_observed_ts = std::max<uint64_t>(
                session_->kudu_latest_observed_ts, latest_kudu_ts);
          }
      

      The required lock acquisition order is SessionState::lock, then QueryExecState::lock_, but the above code acquires them in the opposite order. See IMPALA-4409 for some info on the lock ordering and examples where the locks are acquired in the reverse order.

        Activity

        Hide
        mjacobs Matthew Jacobs added a comment -

        commit 7862e809f608bd34cd0a318d0a91c70a6d95923d
        Author: Matthew Jacobs <mj@cloudera.com>
        Date: Mon Oct 31 16:48:43 2016 -0700

        IMPALA-4411: Kudu inserts violate lock ordering and could deadlock

        This fixes a potential hang because the code took
        QueryExecState::lock_ before SessionState::lock, which is
        the wrong order. A hang has not yet been observed.

        The code that was taking the session lock doesn't actually
        need to be holding the QueryExecState lock_, so this is easy
        to fix by moving the relevant code.

        Testing: Running an exhaustive test job, and stress tests
        manually.

        Change-Id: I2aa36fce78525a80a6b880e1b668105006bc1425
        Reviewed-on: http://gerrit.cloudera.org:8080/4895
        Reviewed-by: Matthew Jacobs <mj@cloudera.com>
        Tested-by: Internal Jenkins

        Show
        mjacobs Matthew Jacobs added a comment - commit 7862e809f608bd34cd0a318d0a91c70a6d95923d Author: Matthew Jacobs <mj@cloudera.com> Date: Mon Oct 31 16:48:43 2016 -0700 IMPALA-4411 : Kudu inserts violate lock ordering and could deadlock This fixes a potential hang because the code took QueryExecState::lock_ before SessionState::lock, which is the wrong order. A hang has not yet been observed. The code that was taking the session lock doesn't actually need to be holding the QueryExecState lock_, so this is easy to fix by moving the relevant code. Testing: Running an exhaustive test job, and stress tests manually. Change-Id: I2aa36fce78525a80a6b880e1b668105006bc1425 Reviewed-on: http://gerrit.cloudera.org:8080/4895 Reviewed-by: Matthew Jacobs <mj@cloudera.com> Tested-by: Internal Jenkins

          People

          • Assignee:
            mjacobs Matthew Jacobs
            Reporter:
            tarmstrong Tim Armstrong
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development