Details
-
Bug
-
Resolution: Fixed
-
Major
-
Lustre 2.1.3, Lustre 2.1.4
-
3
-
6116
Description
We have a lot of nodes with a large amount of unreclaimable memory (over 4GB). Whatever we try to do (manually shrinking the cache, clearing lru locks, ...) the memory can't be recovered. The only way to get the memory back is to umount the lustre filesystem.
After some troubleshooting, I was able to wrote a small reproducer where I just open(2) then close(2) files in O_RDWR (my reproducer use to open thousand of files to emphasize the issue).
Attached 2 programs :
- gentree.c (cc -o gentree gentree.c -lpthread) to generate a tree of known files (no need to use readdir in reproducer.c)
- reproducer.c (cc -o reproducer reproduver.c -lpthread) to reproduce the issue.
The macro BASE_DIR has to be adjust according the local cluster configuration (you should provide the name of a directory located on a lustre filesystem).
There is no link between the 2 phases as rebooting the client between gentree & reproducer does't avoid the problem. Running gentree (which open as much files as reproducer) doesn't show the issue.
Attachments
Issue Links
Activity
Niu, exactly, and I propose to make that 'existing code' able to drop closed open regardless its transno because it doesn't make sense after close. Current solution is still based on hacking server side in various ways. In fact that can be solved at client side, just by letting closed OPENs to be dropped despite their transno.
Andreas, the existing code can only drop open/close request when the last_committed seen by client is bumped, no matter if it's an open_create or not.
If the request is not doing a create, couldn't both the open and close RPC be dropped at this time, regardless of the transno?
Mike, right. I mentioned this (the open request can only be freed when the last_committed on client is bumped) in previous comment, and it's same for the close request. Adding another flag for checking open/close request might be work, what about my solution in http://review.whamcloud.com/6665 ? Could you review it? Thanks.
Niu, Andreas, this become more and more complex as I can see. The problem we are trying to solve is that client keeps closed open requests in replay queue, right? Meanwhile the client itself wants them to be dropped from that queue, see mdc_close():
/* We no longer want to preserve this open for replay even * though the open was committed. b=3632, b=3633 */ spin_lock(&mod->mod_open_req->rq_lock); mod->mod_open_req->rq_replay = 0; spin_unlock(&mod->mod_open_req->rq_lock);
So the problem is that request is not dropped when client ask to do that. That is because of last_committed check which is the only mechanism to drop request and that means for me we just need to add another one to drop request from replay queue regardless its transno. E.g. rq_closed_open flag can be added and checked to drop request. That could be much simpler. Did I miss something and there are other cases we are trying to solve?
Andreas, given that we're going to add MSG flag, I'm thinking that if it would be better to pack another transno in reply. Looks pb_last_seen in ptlrpc_body is not used, I think we can use it to carry the last committed on-disk transo (pb_last_committed stores the last committed on-disk/fake transno), so that we can resolve the issue without introducing duplicate transo. I'll update the patch in this way if it's fine with you. Thanks.
Does this affect the case if the duplicate transnos are < last_committed (i.e. the open/close RPCs)? I thought those ones are just replayed on the server? Ah, you are referencing the client code that replays the RPCs...
If it would make the implementation easier, it would be possible to negotiate between the client and server with MSG_* flags in the ptlrpc_body whether they can handle duplicate transno or not. If not, then the server would have to do occasional commits to bump the transno, otherwise this could be avoided for newer clients & servers.
Per previous comments, the client exp_last_committed should be min(last_committed, max(inode_version of all inodes accessed). That ensures that I'd the client is accessing committed inodes that they can be discarded on close (assuming it isn't the very last file created).
Andreas, my point is that last_committed returned to client can't be larger than the exp_last_committed on disk slot, otherwise, client would think server data lost on next recovery (see recovery-small.sh test_54). So, if a client doesn't update disk, it's exp_last_committed on disk should always be zero, and we can't return last_committed larger than 0 in such case. But I think we may force a disk update in this case.
Another problem is that client code assumes transno unique in some places, such as ptlrpc_replay_next():
cfs_list_for_each_safe(tmp, pos, &imp->imp_replay_list) {
req = cfs_list_entry(tmp, struct ptlrpc_request,
rq_replay_list);
/* If need to resend the last sent transno (because a
reconnect has occurred), then stop on the matching
req and send it again. If, however, the last sent
transno has been committed then we continue replay
from the next request. */
if (req->rq_transno > last_transno) {
if (imp->imp_resend_replay)
lustre_msg_add_flags(req->rq_reqmsg,
MSG_RESENT);
break;
}
req = NULL;
}
If there are duplicate transno requests, some requests will be skipped during replay, so the old client will have trouble with this manner.
Mike, I think there is no way to achieve this without server side changes, I can think of two ways so far:
1. Server treats open/close as committed transactions, and returns client both last committed transno & last real transno (on-disk transno), client drops committed open & close request immediately after close. That's what I did in my patch.
2. Server assigns no transno for open/close, and client open-replay mechanism must be adapted to this change (like Siyao mentioned in the review comment: track open handle in fs layer, and rebuild request when replay the open, and some other changes to open, close, open lock code could be required)
The second solution looks cleaner to me, but it requires more code changes, and it'll be little tricky to handle open-create & open differently on client side.