Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-12751 control: Add a daos filesystem evict command. #12331

Merged
merged 16 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/client/dfuse/dfuse.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ struct dfuse_obj_hdl {
bool doh_kreaddir_started;
/* Set to true if readdir calls reach EOF made on this handle */
bool doh_kreaddir_finished;

bool doh_evict_on_close;
};

/* Readdir support.
Expand Down
5 changes: 5 additions & 0 deletions src/client/dfuse/ops/ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,11 @@ void dfuse_cb_ioctl(fuse_req_t req, fuse_ino_t ino, unsigned int cmd, void *arg,

DFUSE_TRA_DEBUG(oh, "ioctl cmd=%#x", cmd);

if (cmd == DFUSE_IOCTL_DFUSE_EVICT) {
oh->doh_evict_on_close = true;
DFUSE_REPLY_IOCTL_SIZE(oh, req, NULL, 0);
}

if (cmd == DFUSE_IOCTL_COUNT_QUERY) {
handle_cont_query_ioctl(req, in_buf, in_bufsz);
return;
Expand Down
12 changes: 11 additions & 1 deletion src/client/dfuse/ops/open.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ dfuse_cb_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
{
struct dfuse_projection_info *fs_handle = fuse_req_userdata(req);
struct dfuse_inode_entry *ie;
d_list_t *rlink;
d_list_t *rlink;
struct dfuse_obj_hdl *oh = NULL;
struct fuse_file_info fi_out = {0};
int rc;
Expand Down Expand Up @@ -153,5 +153,15 @@ dfuse_cb_release(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
DFUSE_REPLY_ZERO(oh, req);
else
DFUSE_REPLY_ERR_RAW(oh, req, rc);

if (oh->doh_evict_on_close) {
rc = fuse_lowlevel_notify_inval_entry(dfuse_info->di_session, oh->doh_ie->ie_parent,
oh->doh_ie->ie_name,
strnlen(oh->doh_ie->ie_name, NAME_MAX));

if (rc != 0)
DFUSE_TRA_ERROR(oh->doh_ie, "inval_entry() returned: %d (%s)", rc,
strerror(-rc));
}
dfuse_oh_free(dfuse_info, oh);
}
26 changes: 26 additions & 0 deletions src/control/cmd/daos/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type fsCmd struct {
ResetChunkSize fsResetChunkSizeCmd `command:"reset-chunk-size" description:"reset fs chunk size"`
ResetObjClass fsResetOclassCmd `command:"reset-oclass" description:"reset fs obj class"`
DfuseQuery fsDfuseQueryCmd `command:"query" description:"Query dfuse for memory usage"`
DfuseEvict fsDfuseEvictCmd `command:"evict" description:"Evict object from dfuse"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is ONLY for dfuse - not DFS - maybe evict -> dfuse-evict?
Or maybe it's okay if it only makes sense for dfuse anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only thing al these commands have in common is they're for POSIX containers and therefore DFS at some level. evict doesn't mean anything in dfs directly that I'm aware of.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing i was thinking about here is that the daos fs options are on the container (server side), but these dfuse commands apply only on the client side (just the node they are running on). so they are just local operations and only executed on the node where this runs.
i don't know if that counts as grounds that we need to split this off into 2 tools or just have an extension to the dfuse command, or just keep it as it is. what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this extensively in the working group calls, this was your suggestion.

I'd be fine with making a new subcommand for the daos command other than daos filesystem but I don't want to do it in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i know, and i was just rethinking this. im fine to keeping it that way for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should just add a comment that this is a local client operation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree though, there's a clear distinction between commands that have an effect on the container itself vs commands that change local in-memory state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something else to keep in mind is that we currently don't guarantee backwards compatibility for the command line, but I wonder if that will/should be a requirement in the future, since changing the command line interface later can be disruptive to deployment, job setup/cleanup, etc.

}

type fsCopyCmd struct {
Expand Down Expand Up @@ -476,3 +477,28 @@ func (cmd *fsDfuseQueryCmd) Execute(_ []string) error {

return nil
}

type fsDfuseEvictCmd struct {
daosCmd

Args struct {
Path string `positional-arg-name:"path" description:"Path to evict from dfuse" required:"1"`
} `positional-args:"yes"`
}

func (cmd *fsDfuseEvictCmd) Execute(_ []string) error {
ap, deallocCmdArgs, err := allocCmdArgs(cmd.Logger)
if err != nil {
return err
}

ap.path = C.CString(cmd.Args.Path)
defer freeString(ap.path)
defer deallocCmdArgs()

rc := C.dfuse_evict(ap)
if err := daosError(rc); err != nil {
return errors.Wrapf(err, "failed to evict %s", cmd.Args.Path)
}
return nil
}
7 changes: 5 additions & 2 deletions src/include/dfuse_ioctl.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* (C) Copyright 2017-2022 Intel Corporation.
* (C) Copyright 2017-2023 Intel Corporation.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*/
Expand All @@ -26,7 +26,8 @@
#define DFUSE_IOCTL_REPLY_PFILE (DFUSE_IOCTL_REPLY_BASE + 8)

#define DFUSE_IOCTL_R_DFUSE_USER (DFUSE_IOCTL_REPLY_BASE + 9)
#define DFUSE_COUNT_QUERY_CMD (DFUSE_IOCTL_REPLY_BASE + 10)
#define DFUSE_COUNT_QUERY_CMD (DFUSE_IOCTL_REPLY_BASE + 11)
#define DFUSE_IOCTL_EVICT_NR (DFUSE_IOCTL_REPLY_BASE + 11)

/** Metadada caching is enabled for this file */
#define DFUSE_IOCTL_FLAGS_MCACHE (0x1)
Expand Down Expand Up @@ -87,4 +88,6 @@ struct dfuse_mem_query {
#define DFUSE_IOCTL_COUNT_QUERY \
((int)_IOWR(DFUSE_IOCTL_TYPE, DFUSE_COUNT_QUERY_CMD, struct dfuse_mem_query))

#define DFUSE_IOCTL_DFUSE_EVICT ((int)_IO(DFUSE_IOCTL_TYPE, DFUSE_IOCTL_EVICT_NR))

#endif /* __DFUSE_IOCTL_H__ */
83 changes: 82 additions & 1 deletion src/utils/daos_hdlr.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include <sys/xattr.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <fcntl.h>
#include <dlfcn.h>
#include <daos.h>
#include <daos/common.h>
Expand Down Expand Up @@ -2469,3 +2469,84 @@ dfuse_count_query(struct cmd_args_s *ap)
close(fd);
return rc;
}

/* Dfuse cache evict (and helper).
* Open a path and make a ioctl call for dfuse to evict it. IF the path is the root then dfuse
* cannot do this so perform the same over all the top-level directory entries instead.
*/

static int
dfuse_evict_helper(int fd)
{
struct dirent *ent;
DIR *dir;
int rc = 0;

dir = fdopendir(fd);
if (dir == 0) {
rc = errno;
return rc;
}

while ((ent = readdir(dir)) != NULL) {
int cfd;

printf("Processing %s\n", ent->d_name);

cfd = openat(fd, ent->d_name, O_NOFOLLOW, O_RDONLY);
if (cfd < 0) {
rc = errno;
goto out;
}

rc = ioctl(cfd, DFUSE_IOCTL_DFUSE_EVICT);
close(cfd);
if (rc < 0) {
rc = errno;
goto out;
Comment on lines +2507 to +2508
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, shouldn't these use D_GOTO?

}
}

out:
closedir(dir);
return rc;
}

int
dfuse_evict(struct cmd_args_s *ap)
{
struct stat buf;
int rc = -DER_SUCCESS;
int fd;

fd = open(ap->path, O_NOFOLLOW, O_RDONLY);
if (fd < 0) {
rc = errno;
DH_PERROR_SYS(ap, rc, "Failed to open path");
return daos_errno2der(rc);
}

rc = fstat(fd, &buf);
if (rc < 0) {
rc = daos_errno2der(errno);
DH_PERROR_DER(ap, rc, "Failed to stat file");
goto close;
}

if (buf.st_ino == 1) {
rc = daos_errno2der(dfuse_evict_helper(fd));
DH_PERROR_DER(ap, rc, "Unable to traverse root");
goto close;
}

rc = ioctl(fd, DFUSE_IOCTL_DFUSE_EVICT);
if (rc < 0) {
rc = daos_errno2der(errno);
DH_PERROR_DER(ap, rc, "ioctl failed");
goto close;
}

close:
close(fd);
return rc;
}
4 changes: 4 additions & 0 deletions src/utils/daos_hdlr.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ cont_check_hdlr(struct cmd_args_s *ap);
int
cont_clone_hdlr(struct cmd_args_s *ap);

/* Dfuse operations */
int
dfuse_evict(struct cmd_args_s *ap);

/* TODO implement the following container op functions
* all with signatures similar to this:
* int cont_FN_hdlr(struct cmd_args_s *ap)
Expand Down
17 changes: 17 additions & 0 deletions utils/node_local_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2496,6 +2496,23 @@ def test_xattr(self):
for (key, value) in xattr.get_all(fd):
print(f'xattr is {key}:{value}')

@needs_dfuse
def test_evict(self):
"""Evict a file from dfuse"""
new_file = join(self.dfuse.dir, 'e_file')
with open(new_file, 'w'):
pass

rc = run_daos_cmd(self.conf, ['filesystem', 'evict', new_file])
print(rc)
assert rc.returncode == 0, rc
time.sleep(5)

rc = run_daos_cmd(self.conf, ['filesystem', 'evict', self.dfuse.dir])
print(rc)
assert rc.returncode == 0, rc
time.sleep(5)

@needs_dfuse
def test_list_xattr(self):
"""Perform tests with listing extended attributes.
Expand Down