[LU-12125] Allow parallel rename of regular files Created: 28/Aug/15 Updated: 16/Jan/24 Resolved: 21/Apr/21 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | Lustre 2.15.0 |
| Type: | Improvement | Priority: | Minor |
| Reporter: | Andreas Dilger | Assignee: | Andreas Dilger |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Rank (Obsolete): | 9223372036854775807 | ||||||||||||||||||||||||
| Description |
|
In theory it should be possible to relax renaming rules and allow parallel renaming of regular (or non-directory) files.
Probably the easiest is to order such locks by DLM resource (FID+hash) to avoid deadlocks between multiple threads doing renames. For directory renames, they still need to be serialized by the BFL and also take the name hash locks in the source and target dirs to avoid conflict with regular file renames. In theory it would also be possible to allow concurrent rename of directories within the same parent, but that could be done in a separate step. For renames across shards within a single DNE striped directory they would be treated as renames across directories. |
| Comments |
| Comment by Andreas Dilger [ 28/Aug/15 ] |
|
Di, Lai, what do you think of this? How hard would this be to implement? A few lines of code to fix up the locking for regular files? A bunch of code cleanup required first? A major feature to rewrite large parts of the MDS code? In some sense the rename of a regular file could be treated the same as the creation of a file in the target dir and deletion of the file from the source dir, so pdirop locking within the directory would be enough, unlike rename of directories which need full tree locks to avoid creating loops or orphan trees. |
| Comment by Di Wang [ 28/Aug/15 ] |
|
Yes, file rename under the same parent should be easy to implement, (only need fix mdt_rename_lock and order locks of source and target). Hmm, even rename files between directories might be easy to fix, probably we only need consider to order locks of the source and target parents, though with current implementation, we need check the parent/child relationship to order the locks, which will probably still need BFL lock unless we can order the locks by FID + hash, but then we will need change the order of locks everywhere (i.e. from parent-child to FID+hash order), which probably need some time to implement. As you said, directories rename might not be easy, unless we have subtree lock or similar thing. There are still a few simply fixes we can do
|
| Comment by Andreas Dilger [ 05/Sep/19 ] |
|
This might also need the patch https://review.whamcloud.com/14375 " |
| Comment by Gerrit Updater [ 09/Jan/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/41184 |
| Comment by Andreas Dilger [ 09/Jan/21 ] |
|
Lai, do you have a chance to look into this to see how hard it would be to handle same-dir renames (for striped directories only if source and target stripe are the same parent FID) with just a lock on the parent FID? Given that we don't really support interop between different MDS versions, there isn't much concern for protocol interoperability. We might consider an OBD_CONNECT2_RENAME flag to ensure that all of the MDTs have support for this, but even that may not be necessary. At worst, if any of the other MDTs don't support same-dir renames they will still request the LUSTRE_BFL_FID lock from MDT0000 before getting the parent FID locks, which is sub-optimal but is not problematic. Since the same-dir rename will also lock the parent FID, if other MDSes allow this but MDT0000 does not, then at worst the same-dir rename will only be blocked on the directory lock and not LUSTRE_BFL_FID, but this does not affect correctness since there is no lock ordering problem when only renaming regular files. It may be that all is needed is to skip the call to mdt_rename_lock() call if msrcdir == mtgtdir and the renamed object is a regular file. If the client sends the file type with the rename RPC then this is relatively easily done. Unfortunately, that is not the case today: static int ll_rename(struct inode *src, struct dentry *src_dchild, struct inode *tgt, struct dentry *tgt_dchild) { : op_data = ll_prep_md_op_data(NULL, src, tgt, NULL, 0, mode=0, LUSTRE_OPC_ANY, NULL); struct md_op_data *ll_prep_md_op_data(struct md_op_data *op_data, struct inode *i1, struct inode *i2, const char *name, size_t namelen, __u32 mode, enum md_op_code opc, void *data) { : op_data->op_mode = mode; I've pushed a simple patch to send the file mode along with the rename RPC so that the locking decision can be made easily on the MDS. Hopefully this can be included into 2.14 before it is released. |
| Comment by Gerrit Updater [ 09/Jan/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/41186 |
| Comment by Gerrit Updater [ 15/Jan/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/41229 |
| Comment by Gerrit Updater [ 15/Jan/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/41230 |
| Comment by Gerrit Updater [ 15/Jan/21 ] |
|
Andreas Dilger (adilger@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/41231 |
| Comment by Gerrit Updater [ 22/Jan/21 ] |
|
Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/41184/ |
| Comment by Andreas Dilger [ 09/Feb/21 ] |
|
Further improvements in this area beyond the patches already pushed for same-dir rename operations:
I think the benefit of allowing arbitrary parallel directory rename is pretty small, and probably not worthwhile pursuing in the context of this ticket. |
| Comment by Gerrit Updater [ 06/Mar/21 ] |
|
Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/41186/ |
| Comment by Gerrit Updater [ 21/Apr/21 ] |
|
Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/41230/ |
| Comment by Gerrit Updater [ 21/Apr/21 ] |
|
Oleg Drokin (green@whamcloud.com) merged in patch https://review.whamcloud.com/41231/ |
| Comment by Peter Jones [ 21/Apr/21 ] |
|
Looks like everything has landed for 2.15 |