Discussion:
why index (collectionIndex) need a lock?
Nicheal
2014-09-30 07:08:58 UTC
Permalink
Dear develops,

I go through the files (hashIndex, LFNIndex and CollectionIndex), I
cannot find any parameters need to grasp a lock. And basically,
(hashIndex, LFNIndex and CollectionIndex) is used to manage a
collection of objects.

Thus, just in one case, when the spilt or merge is on going, then
lookup() path function is called to get the object path. It may obtain
a wrong path, right?

If so, why not using a state parameter in the class CollectionIndex or
HashIndex indicating the current state of this collection (split,
merge on going or none) and grasp a lock when the state changes. Why
always lock the index before lookup() path or lfn_find()?

Any other reasons?

Nicheal
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Gregory Farnum
2014-09-30 17:23:48 UTC
Permalink
Post by Nicheal
Dear develops,
I go through the files (hashIndex, LFNIndex and CollectionIndex), I
cannot find any parameters need to grasp a lock. And basically,
(hashIndex, LFNIndex and CollectionIndex) is used to manage a
collection of objects.
Thus, just in one case, when the spilt or merge is on going, then
lookup() path function is called to get the object path. It may obtai=
n
Post by Nicheal
a wrong path, right?
If so, why not using a state parameter in the class CollectionIndex o=
r
Post by Nicheal
HashIndex indicating the current state of this collection (split,
merge on going or none) and grasp a lock when the state changes. Why
always lock the index before lookup() path or lfn_find()?
Any other reasons?
Well, you've just named why we have the lock =E2=80=94 it's to prevent
multiple users simultaneously making changes to the *filesystem* that
we can't handle. In particular, no simultaneous ops can run against
the same Collection. This is to prevent situations where a reader
comes in and builds up the path for an object it needs to look at,
then a writer creates a new file which initiates collection split and
the reader's path is suddenly invalid.
-Greg
Software Engineer #42 @ http://inktank.com | http://ceph.com
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Somnath Roy
2014-09-30 17:42:55 UTC
Permalink
Also, I don't think this lock has big impact to performance since it is already sharded to index level. I tried with reader/writer implementation of this lock (logic will be somewhat similar to your state concept) and not getting any benefit .

Thanks & Regards
Somnath

-----Original Message-----
From: ceph-devel-***@vger.kernel.org [mailto:ceph-devel-***@vger.kernel.org] On Behalf Of Gregory Farnum
Sent: Tuesday, September 30, 2014 10:24 AM
To: Nicheal
Cc: ceph-devel
Subject: Re: why index (collectionIndex) need a lock?
Post by Nicheal
Dear develops,
I go through the files (hashIndex, LFNIndex and CollectionIndex), I
cannot find any parameters need to grasp a lock. And basically,
(hashIndex, LFNIndex and CollectionIndex) is used to manage a
collection of objects.
Thus, just in one case, when the spilt or merge is on going, then
lookup() path function is called to get the object path. It may obtain
a wrong path, right?
If so, why not using a state parameter in the class CollectionIndex or
HashIndex indicating the current state of this collection (split,
merge on going or none) and grasp a lock when the state changes. Why
always lock the index before lookup() path or lfn_find()?
Any other reasons?
Well, you've just named why we have the lock — it's to prevent multiple users simultaneously making changes to the *filesystem* that we can't handle. In particular, no simultaneous ops can run against the same Collection. This is to prevent situations where a reader comes in and builds up the path for an object it needs to look at, then a writer creates a new file which initiates collection split and the reader's path is suddenly invalid.
-Greg
Software Engineer #42 @ http://inktank.com | http://ceph.com
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to ***@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

________________________________

PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies).

��{.n�+�������+%��lzwm��b�맲��r��yǩ�ׯzX����ܨ}���Ơz�&j:+v�������zZ+
Noah Watkins
2014-09-30 23:36:09 UTC
Permalink
Post by Somnath Roy
Also, I don't think this lock has big impact to performance since it is already sharded to index level. I tried with reader/writer implementation of this lock (logic will be somewhat similar to your state concept) and not getting any benefit .
If there is interest in identifying locks that are introducing latency
it might useful to add some tracking features to Mutex and RWLock. A
simple thing would be to just record maximum wait times per lock and
dump this via admin socket.
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Milosz Tanski
2014-10-01 03:13:44 UTC
Permalink
Post by Noah Watkins
Post by Somnath Roy
Also, I don't think this lock has big impact to performance since it is already sharded to index level. I tried with reader/writer implementation of this lock (logic will be somewhat similar to your state concept) and not getting any benefit .
If there is interest in identifying locks that are introducing latency
it might useful to add some tracking features to Mutex and RWLock. A
simple thing would be to just record maximum wait times per lock and
dump this via admin socket.
Noah,

You're better off running some kind of synthetic test using mutrace
(you can't use tcmalloc/jemalloc) or measuring futex syscalls via a
pref tracepoint. Generally adding this kind of tracking into the locks
itself ends up being even more expensive.
Post by Noah Watkins
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016

p: 646-253-9055
e: ***@adfin.com
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Noah Watkins
2014-10-01 15:16:57 UTC
Permalink
I didn't know about mutrace, thanks for that reference!
Post by Milosz Tanski
Post by Noah Watkins
Post by Somnath Roy
Also, I don't think this lock has big impact to performance since it is already sharded to index level. I tried with reader/writer implementation of this lock (logic will be somewhat similar to your state concept) and not getting any benefit .
If there is interest in identifying locks that are introducing latency
it might useful to add some tracking features to Mutex and RWLock. A
simple thing would be to just record maximum wait times per lock and
dump this via admin socket.
Noah,
You're better off running some kind of synthetic test using mutrace
(you can't use tcmalloc/jemalloc) or measuring futex syscalls via a
pref tracepoint. Generally adding this kind of tracking into the locks
itself ends up being even more expensive.
Post by Noah Watkins
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Gregory Farnum
2014-10-10 16:44:12 UTC
Permalink
[ Please keep stuff on the list; we don't do private emails and you'll
get faster responses. ]

I don't know for this particular case, but I think it's just an
invariant for all FileStore operations. There's a lot of random stuff
that can go into the omap; it's user-accessible.
-Greg
Thanks, I got your concept.
I find that the Index Ref will be obtained before omap_setkeys (calle=
d
function lfn_find). However, the path is not used in function
omap_setkeys. So why we prevent the Collection (merge or split) when
modifying omap keys? Actually, the information in omap is pg_log,
pg_info, and sometimes, object XATTR, right?
Post by Nicheal
Dear develops,
I go through the files (hashIndex, LFNIndex and CollectionIndex), I
cannot find any parameters need to grasp a lock. And basically,
(hashIndex, LFNIndex and CollectionIndex) is used to manage a
collection of objects.
Thus, just in one case, when the spilt or merge is on going, then
lookup() path function is called to get the object path. It may obt=
ain
Post by Nicheal
a wrong path, right?
If so, why not using a state parameter in the class CollectionIndex=
or
Post by Nicheal
HashIndex indicating the current state of this collection (split,
merge on going or none) and grasp a lock when the state changes. Wh=
y
Post by Nicheal
always lock the index before lookup() path or lfn_find()?
Any other reasons?
Well, you've just named why we have the lock =E2=80=94 it's to preve=
nt
multiple users simultaneously making changes to the *filesystem* tha=
t
we can't handle. In particular, no simultaneous ops can run against
the same Collection. This is to prevent situations where a reader
comes in and builds up the path for an object it needs to look at,
then a writer creates a new file which initiates collection split an=
d
the reader's path is suddenly invalid.
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...