yugabyte / yugabyte-db

YugabyteDB - the cloud native distributed SQL database for mission-critical applications.
https://www.yugabyte.com
Other
8.98k stars 1.07k forks source link

[DocDB] Resume contentious waiter on tserver's rpc threadpool as opposed to the io thread pool #24478

Open basavaraj29 opened 2 weeks ago

basavaraj29 commented 2 weeks ago

Jira Link: DB-13389

Description

We resume waiters from the wait-queue serially. In the process, When resuming a waiter, we try to require the shared in-memory locks with a deadline of 1s. And if this fails, we schedule the resumption of the scheduler which then tries to acquire the shared in-memory locks with deadline set to the rpc requests's deadline itself.

We seem to be using the wrong thread for resumption of such contentious waiters (the ones that couldn't acquire the shared in-memory locks in 1s). In particular, we are currently doing

          messenger_->scheduler().Schedule([serial_waiter](const Status& s) {
            if (!s.ok()) {
              serial_waiter.waiter->InvokeCallbackOrWarn(
                  s.CloneAndPrepend("Failed scheduling contentious waiter resumption: "));
              return;
            }
            ResumeContentiousWaiter(serial_waiter);
          }, std::chrono::milliseconds(100));

which uses

  void HandleTimer(const boost::system::error_code& ec) {
    ...
    auto now = std::chrono::steady_clock::now();
    while (!tasks_.empty() && (*tasks_.begin())->time() <= now) {
      io_service_.post([task = *tasks_.begin()] { task->Run(Status::OK()); });
      ...
    }
    ...
  }

and the ioservice points to a threadpool of size 4.

Messenger::Messenger(const MessengerBuilder &bld)
    : name_(bld.name_),
      ...
      io_thread_pool_(name_, FLAGS_io_thread_pool_size), // defaults to 4

We should have instead used Messenger's normal_thread_pool_ (size of 1024) which is used to execute all rpc requests.

The problematic change was part of this commit. Prior to that, the contentious waiter requests would have just failed with message - unable to acquire locks.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

rthallamko3 commented 2 weeks ago

If we end up using the rpc threadpool, ensure that there are metrics to figure out the following: If there are large number of rpc threadpool workers resuming waiters, then it can negatively impact new reads/writes, resulting in higher read/write latencies.