Project

General

Profile

Actions

Feature #17379

open

rbd: journal: remove the image thread pool size limit

Added by Ricardo Dias over 7 years ago. Updated over 6 years ago.

Status:
New
Priority:
Normal
Assignee:
Target version:
-
% Done:

0%

Source:
other
Tags:
Backport:
Reviewed:
Affected Versions:
Pull request ID:

Description

The current thread pool in ImageCtx.cc is initialized with a size of 1. This should be changed to use the config option "rbd_op_threads".

When making this change, one has to guarantee that the data written in the client cache is written in the same order as in the journal log.

Actions #1

Updated by Venky Shankar over 6 years ago

I ran the rbd teuthology test suite with branch https://github.com/vshankar/ceph/tree/rbd-op-threads along with "rbd op threads: 8" in config yaml.

http://pulpito.ceph.com/vshankar-2017-09-19_06:41:59-rbd-rbd-op-threads-distro-basic-mira/
http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/ (rerun)

Failures can be grouped into 4/5 categories:

- object deleted when still in use (or generally -- asserts in context callbacks due to >1 op threads)

http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/1654745/
http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/1654718/

- I/O error during installation on rbd backed image (possibly issue somewhere in I/O path)

http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/1654716/

- image differ after sync to secondary cluster (there are a bunch of these)

http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/1654737/

Apart from these, there was an assert failure in one of the runs (which passed after rerun) -- backtrace
has `ThreadPool::worker`, but doesn't seem to be due to >1 op threads (needs debugging...)

http://qa-proxy.ceph.com/teuthology/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/1654734/teuthology.log

Actions #2

Updated by Jason Dillaman over 6 years ago

  • Assignee changed from Ricardo Dias to Venky Shankar
Actions #3

Updated by hao jessi over 6 years ago

@Venky Shankar
hi! now i have two questions after saw your log file of rbd teuthology test.
Q1:the exclusive_lock features has been switched off when you ran the test, are you sure the failure is not caused by switching off the exclusive_lock?
Q2: do you think there are some relationships between client cache/snapshot and

1 op threads? is that possible to remove the image thread pool size limit when disable the client cache and snapshot?

Actions #4

Updated by hao jessi over 6 years ago

@Venky Shankar
hi! now i have two questions after saw your log file of rbd teuthology test.
Q1:the exclusive_lock features has been switched off when you ran the test, are you sure the failure is not caused by switching off the exclusive_lock?
Q2: do you think there are some relationships between client cache/snapshot and >1 op threads? is that possible to remove the image thread pool size limit when disable the client cache and snapshot?

Actions #5

Updated by Venky Shankar over 6 years ago

hao jessi wrote:

@Venky Shankar
hi! now i have two questions after saw your log file of rbd teuthology test.
Q1:the exclusive_lock features has been switched off when you ran the test, are you sure the failure is not caused by switching off the exclusive_lock?

Which particular log file are you referring to?

Q2: do you think there are some relationships between client cache/snapshot and

1 op threads? is that possible to remove the image thread pool size limit when disable the client cache and snapshot?

As of now, I'm not sure. There were couple of failed runs which looked like something going awkward in the I/O path due to >1 op threads.

I'll update here when I'm done debugging those.

Actions #6

Updated by Venky Shankar over 6 years ago

Venky Shankar wrote:

I ran the rbd teuthology test suite with branch https://github.com/vshankar/ceph/tree/rbd-op-threads along with "rbd op threads: 8" in config yaml.

http://pulpito.ceph.com/vshankar-2017-09-19_06:41:59-rbd-rbd-op-threads-distro-basic-mira/
http://pulpito.ceph.com/vshankar-2017-09-21_05:52:57-rbd-rbd-op-threads-distro-basic-mira/ (rerun)

Failures can be grouped into 4/5 categories:

- object deleted when still in use (or generally -- asserts in context callbacks due to >1 op threads)
[...]

@Jason Borden: Would we want to fix this with a more generalized approach? afaics, The only problematic portion here is when context callback deletes the object while the object still hasn't done away using (freeing/releasing) resources (other operations such invoking member functions of the object again from the context callback should be okay). The fix, as of now is simply making sure that the resources are not held before calling ->complete().

Apart from these, there was an assert failure in one of the runs (which passed after rerun) -- backtrace
has `ThreadPool::worker`, but doesn't seem to be due to >1 op threads (needs debugging...)
[...]

The above change fixed this too.

Actions #7

Updated by Jason Dillaman over 6 years ago

Perhaps the fix is to have all the state machines derive from RefCountedObject-like class (lighter weight version that doesn't require a "cct" pointer) and tweaking the "create_context_callback" / "create_rados_callback" to perform the reference counting management (and thus deletion).

Actions #8

Updated by Venky Shankar over 6 years ago

Jason Dillaman wrote:

Perhaps the fix is to have all the state machines derive from RefCountedObject-like class (lighter weight version that doesn't require a "cct" pointer) and tweaking the "create_context_callback" / "create_rados_callback" to perform the reference counting management (and thus deletion).

hmmm, I had thought about ref counting, but not at state machine level. Moreover, I had an alternate approach to "fixing" this by ensuring that the provided (incoming to a function) context callback is always completed (invoked) by the thread that created the async context callback (via `create_async_context_callback()`.

Actions #9

Updated by Jason Dillaman over 6 years ago

Note that any solution would also need to support the (eventual) change to librados to support more than a single thread for AIO callbacks (preferably directly invoked from the messenger layer's fast dispatch).

Actions

Also available in: Atom PDF