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.

Sign up for GitHub

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

Jump to bottom

Brick inode table size #4339

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open

Brick inode table size #4339

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a101a42
Raise the network.inode-lru-limit to 64m from 1m.
jkroonza Apr 18, 2024
f64b7ef
Implement network.lru-table-size which sets the size of the inode has…
jkroonza Jul 30, 2023
d9201a3
when dumping state output number of entries in name_hash.
jkroonza Apr 19, 2024
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
3 changes: 3 additions & 0 deletions libglusterfs/src/glusterfs/xlator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,9 @@ get_xlator_by_type(xlator_t *this, char *target);
void
xlator_set_inode_lru_limit(xlator_t *this, void *data);

void
xlator_set_inode_table_size(xlator_t *this, void *data);

int
loc_copy(loc_t *dst, loc_t *src);
int
Expand Down
1 change: 1 addition & 0 deletions libglusterfs/src/libglusterfs.sym
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,7 @@ xlator_option_validate
xlator_option_validate_addr_list
xlator_search_by_name
xlator_set_inode_lru_limit
xlator_set_inode_table_size
xlator_set_type
xlator_set_type_virtual
xlator_subvolume_count
Expand Down
30 changes: 30 additions & 0 deletions libglusterfs/src/xlator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,36 @@ xlator_set_inode_lru_limit(xlator_t *this, void *data)
return;
}

void
xlator_set_inode_table_size(xlator_t *this, void *data)
{
int inode_table_size = 0;

if (this->itable) {
if (!data) {
gf_smsg(this->name, GF_LOG_WARNING, 0, LG_MSG_INPUT_DATA_NULL,
NULL);
goto out;
}
inode_table_size = *(int *)data;

if (inode_table_size != this->itable->inode_hashsize) {
inode_table_t* tmp = inode_table_with_invalidator(this->itable->lru_limit,
this, this->itable->invalidator_fn, this->itable->invalidator_xl,
this->itable->dentry_hashsize, inode_table_size);
if (tmp->inode_hashsize != this->itable->inode_hashsize) {
inode_table_destroy(this->itable);
this->itable = tmp;
} else {
inode_table_destroy(tmp);
}
}
}

out:
return;
}

void
xlator_foreach(xlator_t *this, void (*fn)(xlator_t *each, void *data),
void *data)
Expand Down
3 changes: 3 additions & 0 deletions xlators/mgmt/glusterd/src/glusterd-volume-set.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,9 @@ struct volopt_map_entry glusterd_volopt_map[] = {
{.key = "network.inode-lru-limit",
.voltype = "protocol/server",
.op_version = 1},
{.key = "network.inode-table-size",
.voltype = "protocol/server",
.op_version = GD_OP_VERSION_11_0}, /* is this right? */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^^ other .op_versions here is 1, but this option will only be available from glusterfs 11.2 (or patched 11.1s).

{.key = AUTH_ALLOW_MAP_KEY,
.voltype = "protocol/server",
.option = "!server-auth",
Expand Down
6 changes: 6 additions & 0 deletions xlators/protocol/server/src/server-helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,12 @@ server_build_config(xlator_t *this, server_conf_t *conf)
conf->inode_lru_limit = 16384;
}

ret = dict_get_int32(this->options, "inode-table-size",
&conf->inode_table_size);
if (ret < 0) {
conf->inode_table_size = 65536;
}

data = dict_get(this->options, "trace");
if (data) {
ret = gf_string2boolean(data->data, &conf->trace);
Expand Down
28 changes: 27 additions & 1 deletion xlators/protocol/server/src/server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ server_reconfigure(xlator_t *this, dict_t *options)
rpc_transport_t *xprt = NULL;
rpc_transport_t *xp_next = NULL;
int inode_lru_limit;
int inode_table_size;
gf_boolean_t trace;
data_t *data;
int ret = 0;
Expand Down Expand Up @@ -825,6 +826,22 @@ server_reconfigure(xlator_t *this, dict_t *options)
kid = this;
}

if (dict_get_int32_sizen(options, "inode-table-size", &inode_table_size) ==
0) {
conf->inode_table_size = inode_table_size;
gf_msg_trace(this->name, 0,
"Reconfigured inode-table-size to "
"%d",
conf->inode_table_size);

/* traverse through the xlator graph. For each xlator in the
graph check whether it is a bound_xl or not (bound_xl means
the xlator will have its itable pointer set). If so, then
set the lru limit for the itable.
*/
xlator_foreach(this, xlator_set_inode_table_size, &inode_table_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need to call xlator_foreach here to configure the inode_table_size, you can pass the value(inode_table_size, dentry_hash_size)during call inode_table_new in the function server_setvolume.

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 was following the example from network.inode-lru-limit, will make the required update here, thanks for pointing this out. So just save this into the conf variable then.

}

if (dict_get_int32_sizen(options, "inode-lru-limit", &inode_lru_limit) ==
0) {
conf->inode_lru_limit = inode_lru_limit;
Expand Down Expand Up @@ -1813,12 +1830,21 @@ struct volume_options server_options[] = {
{.key = {"inode-lru-limit"},
.type = GF_OPTION_TYPE_INT,
.min = 0,
.max = 1048576,
.max = 67108864,
.default_value = "16384",
.description = "Specifies the limit on the number of inodes "
"in the lru list of the inode cache.",
.op_version = {1},
.flags = OPT_FLAG_SETTABLE | OPT_FLAG_DOC},
{.key = {"inode-table-size"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to declare a new option inode-table-size, i think inode-table-size is already present, We can pass as an argument inode_table_size to the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you pass that to a brick?

Copy link
Contributor

Choose a reason for hiding this comment

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

for the time being you can pass as an argument to a brick process as like kill the running brick process and start a brick process with same argument and add inode-table-size something like "--inode-table-size=65536" and later you can change glusterd to pass the argument during start of a brick process instead of creating a new option .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So either modify glusterd or add an option? Isn't then just adding an option simpler because that's how most of the rest of the system is managed anyway?

Please note, just trying to understand the logic here.

If adding options to glusterd is the way to go - is there an example mechanism I can follow?

Copy link
Contributor

Choose a reason for hiding this comment

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

So either modify glusterd or add an option? Isn't then just adding an option simpler because that's how most of the rest of the system is managed anyway?

Please note, just trying to understand the logic here.

If adding options to glusterd is the way to go - is there an example mechanism I can follow?

I still believe the best way is to use an existing option(inode-table-size) instead of creating a new argument.To
configure the same via glusterd you can declare an option at glusterd.c option table something like
working-directory and save the configured value in glusterd_conf and access the same value via glusterd_conf_t during brick start in glusterd_volume_start_glusterfs and pass as an argument.

@xhernandez @aravindavk Can you please share your view about the same.

.type = GF_OPTION_TYPE_INT,
.min = 0,
.max = 67108864,
.default_value = "65536",
.description = "Specifies the size of the inode hash table, "
"must be a power of two.",
.op_version = {GD_OP_VERSION_10_0}, /* if possible */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably wrong. Should be 11_0.

.flags = OPT_FLAG_SETTABLE | OPT_FLAG_DOC},
{.key = {"trace"}, .type = GF_OPTION_TYPE_BOOL},
{
.key = {"config-directory", "conf-dir"},
Expand Down
1 change: 1 addition & 0 deletions xlators/protocol/server/src/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct _child_status {
struct server_conf {
rpcsvc_t *rpc;
int inode_lru_limit;
int inode_table_size;
gf_boolean_t server_manage_gids; /* resolve gids on brick */
gf_boolean_t trace;
gf_boolean_t parent_up;
Expand Down